Skip to content

fix(tui): use secure random temp files in external editor to prevent symlink attacks#78

Closed
echobt wants to merge 1 commit intomainfrom
fix/external-editor-symlink-5405
Closed

fix(tui): use secure random temp files in external editor to prevent symlink attacks#78
echobt wants to merge 1 commit intomainfrom
fix/external-editor-symlink-5405

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

Fix symlink attack vulnerability in external editor temp file handling.

Problem

The external editor was using predictable temp filenames based on PID (cortex_prompt_{PID}.md), making it vulnerable to symlink attacks.

Solution

  • Use the tempfile crate with cryptographically random names (16 random bytes)
  • Use O_EXCL flag to fail if file exists (preventing TOCTOU races)
  • Set restrictive permissions (0600 on Unix)

Testing

  • Cargo check passes
  • Both async and sync versions updated

Security Impact

Prevents local privilege escalation via symlink attacks on temp files.

Related Issue

Fixes issue #5405

…symlink attacks

The external editor was using predictable temp filenames based on PID
(cortex_prompt_{PID}.md), making it vulnerable to symlink attacks where
an attacker could:
1. Predict the filename before cortex creates it
2. Create a symlink pointing to a sensitive file
3. When cortex writes to the temp file, it overwrites the symlink target

This fix uses the tempfile crate which:
- Creates files with cryptographically random names (16 random bytes)
- Uses O_EXCL flag to fail if file exists (preventing TOCTOU races)
- Sets restrictive permissions (0600 on Unix)

Changes:
- Replace predictable PID-based filenames with random tempfile names
- Use tempfile::Builder for secure temp file creation
- Update both async and sync versions of open_external_editor
- Add tempfile as a runtime dependency (was already dev dependency)

Security Impact:
Prevents local privilege escalation via symlink attacks on temp files.

Fixes: issue #5405
echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates the following fixes:
- #74: Prevent shell injection in restore_script via path escaping
- #76: Replace unsafe unwrap() with expect() in init_client
- #78: Use secure random temp files in external editor to prevent symlink attacks
- #79: Add per-chunk streaming timeout to prevent indefinite hangs

Key changes:
- Added shell escaping for paths in shell-snapshot restore scripts
- Replaced unwrap() with expect() for better error context in exec runner
- Use secure random temp files instead of predictable names
- Added streaming chunk timeout to prevent hangs during LLM responses
@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

Replaced predictable PID-based temporary filenames with cryptographically secure random names using the tempfile crate to prevent symlink attacks. The implementation:

  • Uses tempfile::Builder with 16 random bytes for unpredictable filenames
  • Automatically applies O_EXCL flag to prevent TOCTOU races
  • Sets restrictive file permissions (0600 on Unix)
  • Updates both async (open_external_editor) and sync (open_external_editor_sync) versions consistently
  • Properly manages temp file lifetime using into_temp_path() to prevent premature deletion

The change addresses a local privilege escalation vulnerability where an attacker could pre-create a symlink at the predictable temp file path to redirect writes to an attacker-controlled location.

Confidence Score: 4/5

  • This PR is safe to merge with one minor consideration around temp file cleanup
  • The security improvement is well-implemented using industry-standard practices. The tempfile crate provides strong guarantees (O_EXCL, secure random names, restrictive permissions). Both async and sync versions are consistently updated. Score of 4 instead of 5 because the reopen() call introduces a minor window where the file could theoretically be accessed between creation and writing, though this is unlikely to be exploitable given the random filename
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-tui/Cargo.toml Added tempfile workspace dependency for secure temp file creation
src/cortex-tui/src/external_editor.rs Replaced predictable PID-based temp files with cryptographically secure random names using tempfile crate with O_EXCL and 0600 permissions

Sequence Diagram

sequenceDiagram
    participant User
    participant App as Cortex TUI
    participant TempFile as tempfile::Builder
    participant FS as File System
    participant Editor as External Editor

    User->>App: Press Ctrl+G with initial content
    App->>TempFile: Builder::new()<br/>.prefix("cortex_prompt_")<br/>.suffix(".md")<br/>.rand_bytes(16)
    TempFile->>FS: Create file with O_EXCL flag<br/>(16 random bytes in name)<br/>permissions: 0600
    FS-->>TempFile: Secure file handle
    TempFile-->>App: tempfile object
    
    App->>TempFile: reopen() for writing
    TempFile-->>App: writable file handle
    App->>FS: write_all(initial_content)
    App->>FS: flush()
    
    App->>App: into_temp_path()<br/>(keep file alive)
    
    App->>Editor: spawn editor process<br/>with temp file path
    Editor->>FS: User edits file
    User->>Editor: Save and exit
    Editor-->>App: Exit status
    
    App->>FS: read_to_string(temp_file)
    FS-->>App: Edited content
    
    App->>FS: remove_file(temp_file)
    App->>User: Return edited content
Loading

@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Consolidated into #82 - fix: consolidated security and robustness improvements

@echobt echobt closed this Feb 4, 2026
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, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +181 to +182
// Keep the temp file alive (don't let it be deleted yet)
let temp_file = temp_file.into_temp_path();
Copy link

Choose a reason for hiding this comment

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

calling into_temp_path() disables automatic cleanup on drop, which could leak temp files if the editor process panics or if there's an early return path. The tempfile would remain in the temp directory until manually cleaned by the OS.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cortex-tui/src/external_editor.rs
Line: 181:182

Comment:
calling `into_temp_path()` disables automatic cleanup on drop, which could leak temp files if the editor process panics or if there's an early return path. The tempfile would remain in the temp directory until manually cleaned by the OS.

How can I resolve this? If you propose a fix, please make it concise.

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