Skip to content

fix(security): consolidated path traversal prevention for MCP and session storage#87

Closed
echobt wants to merge 1 commit intomainfrom
consolidated/path-traversal-security
Closed

fix(security): consolidated path traversal prevention for MCP and session storage#87
echobt wants to merge 1 commit intomainfrom
consolidated/path-traversal-security

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

This PR consolidates 2 security fixes for path traversal vulnerabilities.

Included PRs:

Key Changes:

  • Add sanitize_session_id() function that replaces dangerous characters
  • Add validate_session_id() for pre-validation of untrusted input
  • Add sanitize_server_name() function for MCP server names
  • Add validate_server_name() for pre-validation of MCP server names
  • Only alphanumeric, hyphen, and underscore characters are allowed

Files Modified:

  • src/cortex-tui/src/session/storage.rs
  • src/cortex-tui/src/mcp_storage.rs

Closes #81, #83

…sion storage

## Summary

This PR consolidates **2 security fixes** for path traversal vulnerabilities.

### Included PRs:
- #81: Prevent path traversal in session storage via session_id sanitization
- #83: Prevent path traversal in MCP storage via server name sanitization

### Key Changes:
- Add sanitize_session_id() function that replaces dangerous characters
- Add validate_session_id() for pre-validation of untrusted input
- Add sanitize_server_name() function for MCP server names
- Add validate_server_name() for pre-validation of MCP server names
- Only alphanumeric, hyphen, and underscore characters are allowed

### Files Modified:
- src/cortex-tui/src/session/storage.rs
- src/cortex-tui/src/mcp_storage.rs

Closes #81, #83
@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

This PR consolidates two critical security fixes that prevent path traversal vulnerabilities in both MCP server storage and session storage.

Key Security Improvements:

  • Added sanitize_server_name() and sanitize_session_id() functions that replace all non-alphanumeric characters (except hyphens and underscores) with underscores
  • Added validate_server_name() and validate_session_id() for pre-validation of untrusted input
  • Applied sanitization at the critical path construction points: server_path() in mcp_storage.rs:195 and session_dir() in session/storage.rs:96
  • Comprehensive test coverage for both sanitization and validation functions, including path traversal test cases

Security Analysis:
The implementation effectively prevents path traversal attacks by sanitizing user-controlled input (server names and session IDs) before using them in filesystem operations. The sanitization is applied consistently at the single point where paths are constructed (server_path() and session_dir()), ensuring all downstream operations are protected. This defense-in-depth approach prevents attackers from escaping the intended storage directories using sequences like ../, ../../, or null bytes.

Testing:
All security-critical functions have dedicated unit tests that verify:

  • Normal alphanumeric names pass through unchanged
  • Path traversal attempts (../../../etc) are properly sanitized
  • The resulting paths remain within the intended base directory
  • No .. sequences appear in final paths

Confidence Score: 5/5

  • This PR is safe to merge with no identified issues
  • The security fixes are well-implemented with proper sanitization at the correct entry points, comprehensive test coverage, and no breaking changes to the API. The sanitization functions follow security best practices by using an allowlist approach (only allowing alphanumeric, hyphen, underscore) rather than a denylist. All filesystem operations that use user-controlled input are now protected.
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-tui/src/mcp_storage.rs Added sanitize_server_name() and validate_server_name() functions to prevent path traversal; applied sanitization in server_path(); comprehensive tests included
src/cortex-tui/src/session/storage.rs Added sanitize_session_id() and validate_session_id() functions to prevent path traversal; applied sanitization in session_dir(); comprehensive tests included

Sequence Diagram

sequenceDiagram
    participant User
    participant API as save_server/session_dir
    participant Sanitize as sanitize_*
    participant FS as Filesystem
    
    User->>API: Provide name/ID (potentially malicious)
    Note over User,API: e.g., "../../../etc/passwd"
    
    API->>Sanitize: sanitize_server_name(name) / sanitize_session_id(id)
    Note over Sanitize: Replace non-alphanumeric chars<br/>(except - and _) with _
    Sanitize->>Sanitize: "../../../etc" → "________etc"
    Sanitize-->>API: Return sanitized string
    
    API->>API: Construct path with sanitized value
    Note over API: base_dir.join(sanitized_name)
    
    API->>FS: Access file with safe path
    Note over FS: Path is confined to base_dir<br/>No traversal possible
    FS-->>API: Success
    API-->>User: Operation complete
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.

2 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