fix: consolidated bug fixes and security improvements

[echobt]

Summary

This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change.

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

[greptile-apps]

Greptile Overview

Greptile Summary

This PR consolidates critical security fixes and robustness improvements from multiple previous PRs into a single cohesive change. The changes address security vulnerabilities, prevent resource exhaustion, and improve error handling across the codebase.

Key Changes

Security Hardening

• Path traversal prevention: Sanitized user-controlled inputs (session_id, server_name) before filesystem operations in mcp_storage.rs and session/storage.rs
• Shell injection prevention: Added proper shell escaping for paths in restore scripts using safe quoting techniques
• TOCTOU race condition fixes: Implemented atomic check-and-modify operations in MCP server initialization and plugin registration using write locks and entry API
• Secure temp files: Replaced predictable temp file names with cryptographically secure random names to prevent symlink attacks
• Command validation bypass prevention: Added command normalization to detect attempts to bypass validation using quotes, extra whitespace, or absolute paths

Memory and Resource Management

• Bounded storage: Created ToolResponseStore with configurable size limits and automatic eviction to prevent unbounded memory growth
• Cache size limits: Added eviction policies to token counter cache and stream processor buffers
• Lock cleanup: Implemented automatic cleanup of stale lock entries in file locking manager to prevent memory leaks

Data Durability

• fsync after writes: Added sync_all() calls after critical writes in session storage to prevent data loss on crash or forceful termination
• Directory sync on Unix: Synced parent directories after file operations for crash safety

Error Handling

• Graceful semaphore failures: Changed semaphore operations to return Result instead of unwrapping, preventing panics on unexpected closure
• Safe state transitions: Added validation for tool state transitions with logging for invalid transitions
• Empty collection handling: Made current_section() return Option to handle empty sections gracefully

Numeric Safety

• Saturating arithmetic: Used saturating operations throughout TUI components to prevent overflow on u16 conversions
• Safe UTF-8 slicing: Added boundary-aware string slicing helpers to prevent panics on multi-byte characters

Testing

All changes include comprehensive test coverage, including tests for edge cases like path traversal attempts, quote bypasses, and multi-byte UTF-8 handling.

Confidence Score: 5/5

• This PR is safe to merge with high confidence - all changes are defensive improvements with comprehensive test coverage
• Score reflects the defensive nature of all changes (security hardening, resource limits, error handling), extensive test coverage for edge cases, and clear documentation. No breaking changes or risky refactoring. All fixes follow Rust best practices with proper error propagation.
• No files require special attention - all changes are well-tested security and robustness improvements

Important Files Changed

Filename	Overview
src/cortex-tui/src/mcp_storage.rs	Added path traversal prevention by sanitizing server names before filesystem operations
src/cortex-tui/src/session/storage.rs	Added path traversal prevention and fsync after writes for crash safety and durability
src/cortex-tui/src/external_editor.rs	Replaced predictable temp file names with cryptographically secure random names to prevent symlink attacks
src/cortex-shell-snapshot/src/snapshot.rs	Added shell escaping for paths containing single quotes to prevent shell injection in restore scripts
src/cortex-engine/src/tools/response_store.rs	New bounded storage for tool responses with automatic cleanup to prevent unbounded memory growth
src/cortex-engine/src/validation.rs	Added command normalization to prevent validation bypasses via quotes, paths, and extra whitespace
src/cortex-common/src/file_locking.rs	Added automatic cleanup of stale lock entries to prevent memory leaks in long-running processes
src/cortex-engine/src/streaming.rs	Added buffer size limits and saturating conversions for token counts to prevent overflow and unbounded growth
src/cortex-mcp-server/src/server.rs	Fixed TOCTOU race condition by holding write lock during state check and modification
src/cortex-plugins/src/registry.rs	Fixed TOCTOU race using HashMap entry API for atomic check-and-insert operations

Sequence Diagram

sequenceDiagram
    participant User
    participant TUI
    participant Storage
    participant Engine
    participant MCP
    
    Note over TUI,Storage: Security Layer
    User->>TUI: Request external editor
    TUI->>TUI: Generate cryptographically secure temp file (TOCTOU fix)
    TUI-->>User: Opens editor safely
    
    User->>TUI: Request session save
    TUI->>Storage: Sanitize session ID (path traversal prevention)
    Storage->>Storage: Write with atomic rename
    Storage->>Storage: fsync() for crash safety
    Storage-->>TUI: Session saved durably
    
    User->>Engine: Execute tool with large dataset
    Engine->>Engine: Check ToolResponseStore bounds (MAX_STORE_SIZE)
    Engine->>Engine: Evict oldest if at capacity
    Engine-->>User: Response (memory bounded)
    
    User->>Engine: Stream processing
    Engine->>Engine: Check buffer limits (MAX_BUFFER_SIZE)
    Engine->>Engine: Saturating token conversions
    Engine-->>User: Stream events (overflow prevented)
    
    User->>MCP: Initialize server
    MCP->>MCP: Atomic state check-and-transition (TOCTOU fix)
    MCP-->>User: Server initialized
    
    Note over Engine: Command Validation
    User->>Engine: Execute bash command
    Engine->>Engine: Normalize command (bypass prevention)
    Engine->>Engine: Check against blocked patterns
    alt Command allowed
        Engine-->>User: Execute safely
    else Command blocked
        Engine-->>User: Validation error
    end

Loading

[greptile-apps]

_{10 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}