From e6b880cbcbb1ddd12ff93caced9e9a06cee47506 Mon Sep 17 00:00:00 2001 From: Taylor Eernisse Date: Sun, 8 Feb 2026 07:55:20 -0500 Subject: [PATCH] fix: prevent panics in robot-mode JSON output and arithmetic paths Peer code review found multiple panic-reachable paths: 1. serde_json::to_string().unwrap() in 4 robot-mode output functions (who.rs, main.rs x3). If serialization ever failed (e.g., NaN from edge-case division), the CLI would panic with an unhelpful stack trace. Replaced with unwrap_or_else that emits a structured JSON error fallback. 2. encode_rowid() in chunk_ids.rs used unchecked multiplication (document_id * 1000). On extreme document IDs this could silently wrap in release mode, causing embedding rowid collisions. Now uses checked_mul + checked_add with a diagnostic panic message. 3. HTTP response body truncation at byte index 500 in client.rs could split a multi-byte UTF-8 character, causing a panic. Now uses floor_char_boundary(500) for safe truncation. 4. who.rs reviews mode: SQL used `m.author_username != ?1` which silently dropped MRs with NULL author_username (SQL NULL != anything = NULL). Changed to `(m.author_username IS NULL OR m.author_username != ?1)` to match the pattern already used in expert mode. 5. handle_auth_test hardcoded exit code 5 for all errors regardless of type. Config not found (20), token not set (4), and network errors (8) all incorrectly returned 5. Now uses e.exit_code() from the actual LoreError, with proper suggestion hints in human mode. Co-Authored-By: Claude Opus 4.6 --- src/cli/commands/who.rs | 13 ++++++++---- src/embedding/chunk_ids.rs | 7 ++++++- src/gitlab/client.rs | 12 +++++++---- src/main.rs | 42 +++++++++++++++++++++++++++----------- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/cli/commands/who.rs b/src/cli/commands/who.rs index d6961ec..a33a0fc 100644 --- a/src/cli/commands/who.rs +++ b/src/cli/commands/who.rs @@ -851,7 +851,7 @@ fn query_reviews( WHERE n.author_username = ?1 AND n.note_type = 'DiffNote' AND n.is_system = 0 - AND m.author_username != ?1 + AND (m.author_username IS NULL OR m.author_username != ?1) AND n.created_at >= ?2 AND (?3 IS NULL OR n.project_id = ?3)"; @@ -868,7 +868,7 @@ fn query_reviews( WHERE n.author_username = ?1 AND n.note_type = 'DiffNote' AND n.is_system = 0 - AND m.author_username != ?1 + AND (m.author_username IS NULL OR m.author_username != ?1) AND n.created_at >= ?2 AND (?3 IS NULL OR n.project_id = ?3)"; @@ -888,7 +888,7 @@ fn query_reviews( WHERE n.author_username = ?1 AND n.note_type = 'DiffNote' AND n.is_system = 0 - AND m.author_username != ?1 + AND (m.author_username IS NULL OR m.author_username != ?1) AND ltrim(n.body) LIKE '**%**%' AND n.created_at >= ?2 AND (?3 IS NULL OR n.project_id = ?3) @@ -1798,7 +1798,12 @@ pub fn print_who_json(run: &WhoRun, args: &WhoArgs, elapsed_ms: u64) { meta: RobotMeta { elapsed_ms }, }; - println!("{}", serde_json::to_string(&output).unwrap()); + println!( + "{}", + serde_json::to_string(&output).unwrap_or_else(|e| { + format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#) + }) + ); } #[derive(Serialize)] diff --git a/src/embedding/chunk_ids.rs b/src/embedding/chunk_ids.rs index ea406e5..e834585 100644 --- a/src/embedding/chunk_ids.rs +++ b/src/embedding/chunk_ids.rs @@ -1,7 +1,12 @@ pub const CHUNK_ROWID_MULTIPLIER: i64 = 1000; pub fn encode_rowid(document_id: i64, chunk_index: i64) -> i64 { - document_id * CHUNK_ROWID_MULTIPLIER + chunk_index + document_id + .checked_mul(CHUNK_ROWID_MULTIPLIER) + .and_then(|v| v.checked_add(chunk_index)) + .unwrap_or_else(|| { + panic!("encode_rowid overflow: document_id={document_id}, chunk_index={chunk_index}") + }) } pub fn decode_rowid(rowid: i64) -> (i64, i64) { diff --git a/src/gitlab/client.rs b/src/gitlab/client.rs index 1251e8d..59610c9 100644 --- a/src/gitlab/client.rs +++ b/src/gitlab/client.rs @@ -70,15 +70,19 @@ impl GitLabClient { headers.insert(ACCEPT, HeaderValue::from_static("application/json")); let client = Client::builder() - .default_headers(headers) + .default_headers(headers.clone()) .timeout(Duration::from_secs(30)) .build() .unwrap_or_else(|e| { warn!( error = %e, - "Failed to build configured HTTP client; falling back to default client" + "Failed to build configured HTTP client; falling back to default client with timeout" ); - Client::new() + Client::builder() + .default_headers(headers) + .timeout(Duration::from_secs(30)) + .build() + .unwrap_or_else(|_| Client::new()) }); Self { @@ -180,7 +184,7 @@ impl GitLabClient { let text = response.text().await?; serde_json::from_str(&text).map_err(|e| { let preview = if text.len() > 500 { - &text[..500] + &text[..text.floor_char_boundary(500)] } else { &text }; diff --git a/src/main.rs b/src/main.rs index 3f08290..e18e5c8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -845,7 +845,12 @@ fn print_combined_ingest_json( meta: RobotMeta { elapsed_ms }, }; - println!("{}", serde_json::to_string(&output).unwrap()); + println!( + "{}", + serde_json::to_string(&output).unwrap_or_else(|e| { + format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#) + }) + ); } #[derive(Serialize)] @@ -874,7 +879,12 @@ fn print_combined_dry_run_json( }, }; - println!("{}", serde_json::to_string(&output).unwrap()); + println!( + "{}", + serde_json::to_string(&output).unwrap_or_else(|e| { + format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#) + }) + ); } async fn handle_count( @@ -966,7 +976,12 @@ fn print_init_json(result: &InitResult) { .collect(), }, }; - println!("{}", serde_json::to_string(&output).unwrap()); + println!( + "{}", + serde_json::to_string(&output).unwrap_or_else(|e| { + format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#) + }) + ); } async fn handle_init( @@ -1198,17 +1213,20 @@ async fn handle_auth_test( } Err(e) => { if robot_mode { - let output = FallbackErrorOutput { - error: FallbackError { - code: "AUTH_FAILED".to_string(), - message: e.to_string(), - }, - }; - eprintln!("{}", serde_json::to_string(&output)?); + let output = RobotErrorOutput::from(&e); + eprintln!( + "{}", + serde_json::to_string(&output).unwrap_or_else(|_| { + format!(r#"{{"error":{{"code":"{}","message":"{}"}}}}"#, e.code(), e) + }) + ); } else { - eprintln!("{}", style(format!("Error: {e}")).red()); + eprintln!("{} {}", style("Error:").red(), e); + if let Some(suggestion) = e.suggestion() { + eprintln!("{} {}", style("Hint:").yellow(), suggestion); + } } - std::process::exit(5); + std::process::exit(e.exit_code()); } } }