Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

Fixes #5276 - Lock command session ID slicing panics on multi-byte UTF-8.

Problem

Session ID handling uses byte-based string slicing at 4 locations.

Solution

Replaced with safe slicing using .get() or char-aware methods.

Added a helper function safe_char_prefix that extracts string prefixes by character count rather than byte count, preventing panics on multi-byte UTF-8 characters.

Changes

  • Added safe_char_prefix helper function for UTF-8 safe slicing
  • Fixed 4 locations in is_session_locked, run_list, and run_check functions
  • Added comprehensive unit tests for the helper function

@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

Replaced unsafe byte-based string slicing with character-aware slicing to prevent panics on multi-byte UTF-8 characters in session IDs.

  • Added safe_char_prefix() helper function that uses char_indices() to slice strings by character count rather than byte count
  • Updated 4 locations where [..8.min(len)] slicing was used: is_session_locked() (line 168), run_list() (line 320), and run_check() (lines 344 and 354)
  • Added comprehensive unit tests covering ASCII, multi-byte UTF-8 (emojis, Chinese characters), and boundary cases

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The fix correctly addresses a real UTF-8 panic bug with a proper solution using character-aware slicing, includes comprehensive test coverage for edge cases, and makes no behavioral changes to the lock logic
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-cli/src/lock_cmd.rs Replaced unsafe byte-based slicing with character-aware helper function, added comprehensive UTF-8 tests

Sequence Diagram

sequenceDiagram
    participant User
    participant LockCmd as Lock Command
    participant SafePrefix as safe_char_prefix()
    participant LockFile as Lock File Storage

    Note over User,LockFile: Session ID Display/Matching Flow

    User->>LockCmd: list command
    LockCmd->>LockFile: load_lock_file()
    LockFile-->>LockCmd: locked_sessions[]
    
    loop For each locked session
        LockCmd->>SafePrefix: safe_char_prefix(session_id, 8)
        SafePrefix->>SafePrefix: char_indices().nth(8)
        SafePrefix->>SafePrefix: slice at byte boundary
        SafePrefix-->>LockCmd: safe 8-char prefix
        LockCmd->>User: display short_id
    end

    User->>LockCmd: check/is_locked(session_id)
    LockCmd->>LockFile: load_lock_file()
    LockFile-->>LockCmd: locked_sessions[]
    
    loop For each entry
        LockCmd->>SafePrefix: safe_char_prefix(entry.session_id, 8)
        SafePrefix-->>LockCmd: safe 8-char prefix
        LockCmd->>LockCmd: starts_with() check
    end
    
    LockCmd-->>User: lock status result
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates the following UTF-8 safety fixes:
- #31: Use safe UTF-8 slicing in import command base64 extraction
- #32: Use safe UTF-8 slicing for session IDs in notifications
- #33: Use char-aware string truncation for UTF-8 safety in resume
- #35: Use safe UTF-8 slicing for session IDs in lock command
- #37: Validate UTF-8 boundaries in mention parsing

All changes ensure safe string operations that respect UTF-8 boundaries:
- Replaced direct byte slicing with char-aware methods
- Added floor_char_boundary checks before slicing
- Prevents panics from slicing multi-byte characters
@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Consolidated into #70 - fix: consolidated UTF-8 safety improvements for string slicing

@echobt echobt closed this Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant