Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

This PR consolidates 3 numeric safety fixes into a single, cohesive change.

Included PRs:

Changes:

  • Replaced direct casts with saturating_sub and try_into
  • Prevents panic on numeric overflow/underflow conditions
  • Uses safe arithmetic throughout

Files Modified:

  • src/cortex-compact/src/compactor.rs
  • src/cortex-engine/src/git_info.rs
  • src/cortex-engine/src/streaming.rs

Closes #39, #40, #41

…ng operations

This PR consolidates the following numeric safety fixes:
- #39: Use saturating casts in git_info to prevent overflow
- #40: Use saturating casts for token counts in streaming
- #41: Use saturating subtraction to prevent underflow in compaction

All changes use saturating arithmetic operations:
- Replaced direct casts with saturating_sub and try_into
- Prevents panic on numeric overflow/underflow conditions
@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

This PR consolidates three numeric safety fixes to prevent overflow and underflow conditions across different components:

Changes Made:

  • compactor.rs: Replaced direct arithmetic with saturating_sub and saturating_add when calculating token counts after compaction, preventing underflow when tokens_in_compacted exceeds current_tokens
  • git_info.rs: Added saturation logic using .min(u32::MAX as usize) before casting usize counts to u32 for both file changes and stash indices, preventing truncation overflow for large repositories
  • streaming.rs: Implemented saturating_i64_to_u32 helper function to safely convert i64 token counts to u32, handling negative values (clamp to 0) and values exceeding u32::MAX (clamp to max)

Impact:
All changes are defensive improvements that prevent panic conditions. The saturating behavior provides graceful degradation - in edge cases where values would overflow/underflow, the system will use boundary values (0 or MAX) instead of crashing.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • All changes are defensive improvements that replace potentially unsafe operations with saturating alternatives. The changes are well-targeted, properly implemented, and add robustness without altering intended behavior in normal cases.
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-compact/src/compactor.rs Replaced arithmetic with saturating operations to prevent underflow when calculating tokens_after
src/cortex-engine/src/git_info.rs Added saturation when casting usize line counts to u32 to prevent truncation overflow
src/cortex-engine/src/streaming.rs Implemented saturating_i64_to_u32 helper to safely convert token counts with bounds checking

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant Comp as Compactor
    participant Git as GitInfo
    participant Stream as Streaming
    
    Note over App,Stream: Numeric Overflow/Underflow Prevention

    App->>Comp: compact(items, summary_text, current_tokens)
    Note over Comp: tokens_in_compacted may exceed current_tokens
    Comp->>Comp: saturating_sub(current_tokens, tokens_in_compacted)
    Note over Comp: Prevents underflow: min value = 0
    Comp->>Comp: saturating_add(result, summary_tokens)
    Note over Comp: Prevents overflow: max value = usize::MAX
    Comp-->>App: CompactionResult with safe tokens_after

    App->>Git: GitInfo::from_path(path)
    Git->>Git: count status lines (returns usize)
    Note over Git: Line count may exceed u32::MAX
    Git->>Git: min(count, u32::MAX as usize) as u32
    Note over Git: Saturates to u32::MAX if too large
    Git-->>App: GitInfo with safe u32 counts

    App->>Stream: Convert TokenUsage (i64) to StreamTokenUsage (u32)
    Note over Stream: Token values may be negative or > u32::MAX
    Stream->>Stream: saturating_i64_to_u32(input_tokens)
    alt value <= 0
        Stream->>Stream: Return 0
    else value > u32::MAX
        Stream->>Stream: Return u32::MAX
    else valid range
        Stream->>Stream: Return value as u32
    end
    Stream-->>App: StreamTokenUsage with safe u32 tokens
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.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Closing this PR to consolidate into a single mega-PR combining all bug fixes. The changes will be included in a new consolidated PR.

@echobt echobt closed this Feb 4, 2026
echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change.

## Categories

### Security Fixes
- Path traversal prevention in MCP and session storage
- Shell injection prevention in restore scripts
- Secure random temp files for external editor
- TOCTOU race condition fixes

### TUI Improvements
- Overflow prevention for u16 conversions
- Cursor positioning fixes in selection lists
- Unicode width handling for popups
- Empty section handling in help browser

### Error Handling
- Graceful semaphore and init failure handling
- Improved error propagation in middleware
- Better client access error handling
- SystemTime operation safety

### Memory and Storage
- Cache size limits to prevent unbounded growth
- File lock cleanup for memory leak prevention
- fsync after critical writes for durability
- Bounded ToolResponseStore with automatic cleanup

### Protocol Robustness
- Buffer size limits for StreamProcessor
- ToolState transition validation
- State machine documentation

### Numeric Safety
- Saturating operations to prevent overflow/underflow
- Safe UTF-8 string slicing throughout codebase

### Tools
- Parameter alias support for backward compatibility
- Handler name consistency fixes

## Files Modified
Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common,
cortex-protocol, cortex-storage, cortex-mcp-server, and other crates.

Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change.

## Categories

### Security Fixes
- Path traversal prevention in MCP and session storage
- Shell injection prevention in restore scripts
- Secure random temp files for external editor
- TOCTOU race condition fixes

### TUI Improvements
- Overflow prevention for u16 conversions
- Cursor positioning fixes in selection lists
- Unicode width handling for popups
- Empty section handling in help browser

### Error Handling
- Graceful semaphore and init failure handling
- Improved error propagation in middleware
- Better client access error handling
- SystemTime operation safety

### Memory and Storage
- Cache size limits to prevent unbounded growth
- File lock cleanup for memory leak prevention
- fsync after critical writes for durability
- Bounded ToolResponseStore with automatic cleanup

### Protocol Robustness
- Buffer size limits for StreamProcessor
- ToolState transition validation
- State machine documentation

### Numeric Safety
- Saturating operations to prevent overflow/underflow
- Safe UTF-8 string slicing throughout codebase

### Tools
- Parameter alias support for backward compatibility
- Handler name consistency fixes

## Files Modified
Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common,
cortex-protocol, cortex-storage, cortex-mcp-server, and other crates.

Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
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