Skip to content

fix(cli): guard six byte-indexed slice sites against multibyte/overrun panics - #3451

Open
yassine-ceo wants to merge 1 commit into
rtk-ai:developfrom
yassine-ceo:fix/byte-indexed-slice-panics
Open

yassine-ceo wants to merge 1 commit into
rtk-ai:developfrom
yassine-ceo:fix/byte-indexed-slice-panics

Conversation

@yassine-ceo

Copy link
Copy Markdown

Summary

Fixes a family of byte-indexed string slices that can panic on multi-byte input, plus one out-of-bounds overrun. Regression tests were added before each fix (TDD, Red-Green) and each test fails on the pre-fix code.

Byte-boundary panics (sliced mid-character)

Out-of-bounds overrun

Testing

  • cargo test — 2549 passed; 16 failed are pre-existing core::stream tests that spawn Unix-only binaries (sh, true, false, echo, cat) and cannot run on Windows (they run fine in Linux CI and are untouched by this PR).
  • cargo fmt --all --check — clean
  • cargo clippy --all-targets — clean

All six new regression tests fail on the pre-fix code and pass after.

…n 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 (rtk-ai#3444)
- prisma_cmd.rs: span a line-final migration token by the rest of the line, not a fixed 20 bytes (rtk-ai#3445)
- find_cmd.rs: shorten_dir_display keeps trailing 47 bytes on a char boundary (rtk-ai#3415)
- git.rs: parse_commit_output strips the leading '[' via strip_prefix so byte 1 is always a boundary (rtk-ai#3415)
- gain.rs: short_month_day rounds the 5..10 slice to char boundaries (same class as rtk-ai#3415)
- display_helpers.rs: WeekStats::period rounds the 5.. slice to char boundaries (same class as rtk-ai#3415)

TDD: regression tests added for all six sites before each fix.
@CLAassistant

CLAassistant commented Aug 5, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

dev1abz added a commit to dev1abz/rtk that referenced this pull request Aug 7, 2026
Overlapping git.rs/find_cmd.rs sites resolved in favour of rtk-ai#3435, which
applies the same strip_prefix fix; rtk-ai#3451 contributes the remaining four
sites (gain, prisma, display_helpers, utils).
@KuSh

KuSh commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Three of the six sites in this PR are already fixed on develop: parse_commit_output was rewritten by commit f496f59 (2026-08-15), find's directory label truncates by chars since #3603 (merged 2026-08-26), and filter_migrate_status got its fallback in #3836 (merged 2026-09-11). What develop still lacks are the other three byte-indexed slices and their regression tests: truncate_iso_date's &date[..10] in src/core/utils.rs, the &date[5..10] in the gain ASCII graph in src/analytics/gain.rs, and the two &s[5..] slices in WeekStats::period in src/core/display_helpers.rs. Could you rebase onto current develop and narrow the PR to those three? It conflicts with develop, partly because src/cmds/git/git.rs has since been renamed, and that hunk can simply be dropped. Thanks for the careful audit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants