From f6253ad776b6c728a732d6a0fdc08f428d7197ea Mon Sep 17 00:00:00 2001 From: yassine-ceo <105609916+yassine-ceo@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:56:30 +0100 Subject: [PATCH] fix(cli): guard six byte-indexed slice sites against multibyte/overrun panics Harden byte-indexed string slices that could panic on multi-byte input or when a trailing token is shorter than the fallback span. - utils.rs: round the 10-byte truncate_iso_date cut down to a char boundary (#3444) - prisma_cmd.rs: span a line-final migration token by the rest of the line, not a fixed 20 bytes (#3445) - find_cmd.rs: shorten_dir_display keeps trailing 47 bytes on a char boundary (#3415) - git.rs: parse_commit_output strips the leading '[' via strip_prefix so byte 1 is always a boundary (#3415) - gain.rs: short_month_day rounds the 5..10 slice to char boundaries (same class as #3415) - display_helpers.rs: WeekStats::period rounds the 5.. slice to char boundaries (same class as #3415) TDD: regression tests added for all six sites before each fix. --- src/analytics/gain.rs | 37 +++++++++++++++++++++++++++++++++- src/cmds/git/git.rs | 32 +++++++++++++++++++++++------ src/cmds/js/prisma_cmd.rs | 15 +++++++++++++- src/cmds/system/find_cmd.rs | 40 ++++++++++++++++++++++++++++++++----- src/core/display_helpers.rs | 24 ++++++++++++++++++++-- src/core/utils.rs | 22 +++++++++++++++----- 6 files changed, 150 insertions(+), 20 deletions(-) diff --git a/src/analytics/gain.rs b/src/analytics/gain.rs index 980c04738f..72f7d18f56 100644 --- a/src/analytics/gain.rs +++ b/src/analytics/gain.rs @@ -452,6 +452,16 @@ fn shorten_path(path: &str) -> String { } } +/// Shorten an ISO day stamp to its month-day portion (chars 5..10), rounding +/// down to char boundaries so multi-byte input can never panic. +fn short_month_day(date: &str) -> &str { + if date.len() >= 10 { + &date[date.floor_char_boundary(5)..date.floor_char_boundary(10)] + } else { + date + } +} + fn print_ascii_graph(data: &[(String, usize)]) { if data.is_empty() { return; @@ -461,7 +471,7 @@ fn print_ascii_graph(data: &[(String, usize)]) { let width = 40; for (date, value) in data { - let date_short = if date.len() >= 10 { &date[5..10] } else { date }; + let date_short = short_month_day(date); let bar_len = if max_val > 0 { ((*value as f64 / max_val as f64) * width as f64) as usize @@ -760,3 +770,28 @@ fn confirm_reset() -> Result { Ok(matches!(line.trim().to_lowercase().as_str(), "y" | "yes")) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn short_month_day_ascii() { + assert_eq!(short_month_day("2026-08-05"), "08-05"); + } + + #[test] + fn short_month_day_short_input_untouched() { + assert_eq!(short_month_day("x"), "x"); + } + + /// Regression test (#3415 class): a multi-byte character straddling byte 5 + /// made `&date[5..10]` slice mid-character and panic; the slice now rounds + /// down to char boundaries. + #[test] + fn short_month_day_multibyte_never_panics() { + let s = "2024\u{00e9}1-20"; // 'é' occupies bytes 4..6 + let result = short_month_day(s); + assert!(std::str::from_utf8(result.as_bytes()).is_ok()); + } +} diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index 9936a061d8..3a36441050 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -1009,12 +1009,18 @@ fn build_commit_command(args: &[String], global_args: &[String]) -> Command { /// Handles: `[main abc1234def] message`, `[main (root-commit) abc1234def] msg`, /// localized variants, and multibyte branch names. fn parse_commit_output(line: &str) -> String { - if let Some(bracket_end) = line.find(']') { - let bracket_content = &line[1..bracket_end]; - let hash = bracket_content.split_whitespace().next_back().unwrap_or(""); - if !hash.is_empty() && hash.len() >= 7 { - let short_hash: String = hash.chars().take(7).collect(); - format!("ok {}", short_hash) + // `strip_prefix` (rather than `&line[1..]`) guarantees the leading `[` is + // really there and that byte 1 is a char boundary for multibyte prefixes. + if let Some(rest) = line.strip_prefix('[') { + if let Some(bracket_end) = rest.find(']') { + let bracket_content = &rest[..bracket_end]; + let hash = bracket_content.split_whitespace().next_back().unwrap_or(""); + if !hash.is_empty() && hash.len() >= 7 { + let short_hash: String = hash.chars().take(7).collect(); + format!("ok {}", short_hash) + } else { + "ok".to_string() + } } else { "ok".to_string() } @@ -2869,6 +2875,20 @@ no changes added to commit (use "git add" and/or "git commit -a") assert_eq!(parse_commit_output(""), "ok"); } + /// Regression test (#3415): a line beginning with `]` made `&line[1..0]` + /// an empty backwards range and panicked. + #[test] + fn test_parse_commit_output_starts_with_closing_bracket() { + assert_eq!(parse_commit_output("] done"), "ok"); + } + + /// Regression test (#3415): a multi-byte first character made byte 1 not + /// a char boundary, so `&line[1..]` panicked. + #[test] + fn test_parse_commit_output_multibyte_first_char() { + assert_eq!(parse_commit_output("日本] x"), "ok"); + } + // --- commit outcome classification (issue #2494) --- #[test] diff --git a/src/cmds/js/prisma_cmd.rs b/src/cmds/js/prisma_cmd.rs index 3deef1c6f9..0ea7d6049d 100644 --- a/src/cmds/js/prisma_cmd.rs +++ b/src/cmds/js/prisma_cmd.rs @@ -316,7 +316,10 @@ fn filter_migrate_status(output: &str) -> String { applied_count += 1; if latest_migration.is_empty() && line.contains("202") { if let Some(pos) = line.find("202") { - let end = line[pos..].find(|c: char| c.is_whitespace()).unwrap_or(20); + // Same fallback as the migrate-dev path above: when the + // migration token is line-final, the span is the rest of + // the line, never a fixed 20 bytes (issue #3445). + let end = line[pos..].find(|c: char| c.is_whitespace()).unwrap_or(line.len() - pos); latest_migration = line[pos..pos + end].to_string(); } } @@ -488,4 +491,14 @@ CREATE INDEX "session_status_idx" ON "Session"("status"); assert_eq!(extract_number("42 models generated"), Some(42)); assert_eq!(extract_number("no numbers here"), None); } + + /// Regression test (#3445): a migration token at end-of-line shorter than + /// 20 bytes used the `.unwrap_or(20)` fallback and sliced past the end of + /// the string. + #[test] + fn test_filter_migrate_status_line_final_migration_never_panics() { + let output = "applied 20240115_x"; + let result = filter_migrate_status(output); + assert!(result.contains("Latest: 20240115_x"), "got: {result}"); + } } diff --git a/src/cmds/system/find_cmd.rs b/src/cmds/system/find_cmd.rs index 0e8d2eebb9..d2e641f93f 100644 --- a/src/cmds/system/find_cmd.rs +++ b/src/cmds/system/find_cmd.rs @@ -177,6 +177,16 @@ fn parse_rtk_find_args(args: &[String]) -> Result { Ok(parsed) } +/// Shorten a directory for display: keep the trailing 47 bytes (rounded down +/// to a char boundary) with a "..." prefix when the path exceeds 50 bytes. +fn shorten_dir_display(dir: &str) -> String { + if dir.len() > 50 { + format!("...{}", &dir[dir.floor_char_boundary(dir.len() - 47)..]) + } else { + dir.to_string() + } +} + /// Entry point from main.rs — parses raw args then delegates to run(). pub fn run_from_args(args: &[String], verbose: u8) -> Result<()> { let parsed = parse_find_args(args)?; @@ -322,11 +332,7 @@ pub fn run( } let files_in_dir = &by_dir[dir]; - let dir_display = if dir.len() > 50 { - format!("...{}", &dir[dir.len() - 47..]) - } else { - dir.clone() - }; + let dir_display = shorten_dir_display(dir); let remaining_budget = max_results - displayed; if files_in_dir.len() <= remaining_budget { @@ -617,4 +623,28 @@ mod tests { // We can't easily capture stdout in unit tests, but at least // verify it runs without error. The smoke tests verify content. } + + // --- shorten_dir_display (issue #3415) --- + + /// Regression test (#3415): a path longer than 50 bytes whose byte at + /// `len - 47` falls inside a multi-byte character (20 Thai chars = 60 + /// bytes) panicked; the tail must start on a char boundary. + #[test] + fn shorten_dir_display_multibyte_never_panics() { + let cjk = "ก".repeat(20); + let result = shorten_dir_display(&cjk); + assert!(result.starts_with("...")); + assert!(std::str::from_utf8(result.as_bytes()).is_ok()); + } + + #[test] + fn shorten_dir_display_keeps_last_47_bytes_ascii() { + let ascii = "a".repeat(60); + assert_eq!(shorten_dir_display(&ascii), format!("...{}", "a".repeat(47))); + } + + #[test] + fn shorten_dir_display_short_path_untouched() { + assert_eq!(shorten_dir_display("src/cmds"), "src/cmds"); + } } diff --git a/src/core/display_helpers.rs b/src/core/display_helpers.rs index affc331bff..7977e65001 100644 --- a/src/core/display_helpers.rs +++ b/src/core/display_helpers.rs @@ -202,12 +202,12 @@ impl PeriodStats for WeekStats { fn period(&self) -> String { let start = if self.week_start.len() > 5 { - &self.week_start[5..] + &self.week_start[self.week_start.floor_char_boundary(5)..] } else { &self.week_start }; let end = if self.week_end.len() > 5 { - &self.week_end[5..] + &self.week_end[self.week_end.floor_char_boundary(5)..] } else { &self.week_end }; @@ -346,6 +346,26 @@ mod tests { assert_eq!(WeekStats::label(), "Weekly"); } + /// Regression test (#3415 class): a multi-byte character before byte 5 in + /// week_start/week_end made `&s[5..]` slice mid-character and panic. + #[test] + fn test_week_stats_trait_multibyte_never_panics() { + let week = WeekStats { + week_start: "2024\u{00e9}1-20".to_string(), + week_end: "2024\u{00e9}1-26".to_string(), + commands: 1, + input_tokens: 1, + output_tokens: 1, + saved_tokens: 1, + savings_pct: 0.0, + total_time_ms: 1, + avg_time_ms: 1, + }; + + let period = week.period(); + assert!(std::str::from_utf8(period.as_bytes()).is_ok()); + } + #[test] fn test_month_stats_trait() { let month = MonthStats { diff --git a/src/core/utils.rs b/src/core/utils.rs index d10f4b0118..6d9b0f839e 100644 --- a/src/core/utils.rs +++ b/src/core/utils.rs @@ -166,11 +166,9 @@ pub fn join_with_overflow(items: &[String], total: usize, max: usize, label: &st /// assert_eq!(truncate_iso_date("short"), "short"); /// ``` pub fn truncate_iso_date(date: &str) -> &str { - if date.len() >= 10 { - &date[..10] - } else { - date - } + // Round the 10-byte cut down to a char boundary: a multi-byte character + // straddling byte 10 must not be sliced through (issue #3444). + &date[..date.floor_char_boundary(10.min(date.len()))] } /// Format a confirmation message: "ok \ \" @@ -689,6 +687,20 @@ mod tests { assert_eq!(result, "rtk ls -la Ародинамиче..."); } + // ===== truncate_iso_date multibyte safety (issue #3444) ===== + + /// Regression test (#3444): `&date[..10]` sliced inside a multi-byte + /// character ('é' occupies bytes 9..11) and panicked; the prefix must + /// land on a char boundary instead. + #[test] + fn test_truncate_iso_date_multibyte_never_panics() { + let s = "2024-01-1\u{00e9}5T10:30:00Z"; + assert_eq!(truncate_iso_date(s), "2024-01-1"); + // Existing behavior preserved for plain ASCII and short strings. + assert_eq!(truncate_iso_date("2024-01-15T10:30:00Z"), "2024-01-15"); + assert_eq!(truncate_iso_date("short"), "short"); + } + // ===== resolve_binary tests (issue #212) ===== #[test]