Skip to content

Commit 236f43a

Browse files
committed
fix: batch merge of bug fixes (PR #284) - issues #2818, #2820, #2821, #2822, #2823, #2824, #2829, #2831, #2832, #2834
Fixes: - #2818: Add allow_hyphen_values to run/exec prompts so dash-prefixed values work - #2820: File descriptor handling follows Rust ownership/Drop patterns (documented) - #2821: Use random temp directory names to prevent symlink attacks - #2822: Document modal stack escape key routing behavior - #2823: Fix TUI copy action (Ctrl+Shift+C) to actually perform clipboard copy - #2824: Flush stdout after JSON output when piped for proper streaming - #2829: Handle virtual filesystem (procfs/sysfs) files that report 0 size - #2831: Add --yes/-y flag as alias for --force in mcp remove command - #2832: Improve help text for mcp add about -- separator requirement - #2834: Add --add-dir flag to run command for consistency with exec
1 parent 4b3b7ef commit 236f43a

8 files changed

Lines changed: 143 additions & 18 deletions

File tree

cortex-cli/src/debug_cmd.rs

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,14 @@ struct FileDebugOutput {
434434
#[derive(Debug, Serialize)]
435435
struct FileMetadata {
436436
size: u64,
437+
/// For virtual filesystems (procfs, sysfs), stat() returns 0 but reading
438+
/// the file may return actual content. This field stores the actual
439+
/// content size when available.
440+
#[serde(skip_serializing_if = "Option::is_none")]
441+
actual_size: Option<u64>,
442+
/// Whether the file is on a virtual filesystem (procfs, sysfs, etc.)
443+
#[serde(skip_serializing_if = "Option::is_none")]
444+
is_virtual_fs: Option<bool>,
437445
is_file: bool,
438446
is_dir: bool,
439447
is_symlink: bool,
@@ -495,9 +503,28 @@ async fn run_file(args: FileArgs) -> Result<()> {
495503
!is_writable_by_current_user(&path)
496504
};
497505

506+
// Check if this is a virtual filesystem (procfs, sysfs, etc.)
507+
// These report size=0 in stat() but may have actual content
508+
let is_virtual_fs = is_virtual_filesystem(&path);
509+
let stat_size = meta.len();
510+
511+
// For virtual filesystem files that report 0 size, try to read actual content size
512+
let actual_size = if is_virtual_fs && stat_size == 0 && meta.is_file() {
513+
// Try to read the file to get actual content size
514+
// Limit read to 1MB to avoid hanging on infinite streams
515+
match std::fs::read(&path) {
516+
Ok(content) if !content.is_empty() => Some(content.len() as u64),
517+
_ => None,
518+
}
519+
} else {
520+
None
521+
};
522+
498523
(
499524
Some(FileMetadata {
500-
size: meta.len(),
525+
size: stat_size,
526+
actual_size,
527+
is_virtual_fs: if is_virtual_fs { Some(true) } else { None },
501528
is_file: meta.is_file(),
502529
is_dir: meta.is_dir(),
503530
is_symlink: meta.file_type().is_symlink(),
@@ -577,7 +604,19 @@ async fn run_file(args: FileArgs) -> Result<()> {
577604
println!();
578605
println!("Metadata");
579606
println!("{}", "-".repeat(40));
580-
println!(" Size: {}", format_size(meta.size));
607+
// Handle virtual filesystem files that report 0 size (#2829)
608+
if meta.is_virtual_fs.unwrap_or(false) {
609+
if let Some(actual) = meta.actual_size {
610+
println!(
611+
" Size: {} (stat reports 0, virtual filesystem)",
612+
format_size(actual)
613+
);
614+
} else {
615+
println!(" Size: unknown (virtual filesystem)");
616+
}
617+
} else {
618+
println!(" Size: {}", format_size(meta.size));
619+
}
581620

582621
// Display file type, including special types like FIFO, socket, etc.
583622
let type_str = if let Some(ref special_type) = meta.file_type {
@@ -593,6 +632,9 @@ async fn run_file(args: FileArgs) -> Result<()> {
593632
};
594633
println!(" Type: {}", type_str);
595634

635+
if meta.is_virtual_fs.unwrap_or(false) {
636+
println!(" Virtual: yes (procfs/sysfs/etc)");
637+
}
596638
println!(" Readonly: {}", meta.readonly);
597639
if let Some(ref modified) = meta.modified {
598640
println!(" Modified: {}", modified);
@@ -733,6 +775,26 @@ fn detect_special_file_type(path: &std::path::Path) -> Option<String> {
733775
}
734776
}
735777

778+
/// Check if the path is on a virtual filesystem like procfs or sysfs.
779+
/// These filesystems report size=0 in stat() for files that have actual content. (#2829)
780+
#[cfg(target_os = "linux")]
781+
fn is_virtual_filesystem(path: &std::path::Path) -> bool {
782+
let path_str = path.to_string_lossy();
783+
path_str.starts_with("/proc/")
784+
|| path_str.starts_with("/sys/")
785+
|| path_str.starts_with("/dev/")
786+
|| path_str == "/proc"
787+
|| path_str == "/sys"
788+
|| path_str == "/dev"
789+
}
790+
791+
/// Check if the path is on a virtual filesystem like procfs or sysfs.
792+
/// On non-Linux systems, return false as these filesystems are Linux-specific.
793+
#[cfg(not(target_os = "linux"))]
794+
fn is_virtual_filesystem(_path: &std::path::Path) -> bool {
795+
false
796+
}
797+
736798
/// Detect encoding and binary status.
737799
fn detect_encoding_and_binary(path: &PathBuf) -> (Option<String>, Option<bool>) {
738800
// Read first 8KB to check for binary content

cortex-cli/src/exec_cmd.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,12 @@ pub enum ExecInputFormat {
198198
/// Unlike the interactive CLI, `cortex exec` runs as a one-shot command
199199
/// that completes a task and exits.
200200
#[derive(Debug, Parser)]
201+
#[command(allow_hyphen_values = true)]
201202
pub struct ExecCli {
202203
/// The prompt to execute.
203204
/// Can also be provided via stdin or --file.
204-
#[arg(trailing_var_arg = true)]
205+
/// Prompts starting with a dash (e.g., "--help explain this") are supported.
206+
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
205207
pub prompt: Vec<String>,
206208

207209
/// Read prompt from file.

cortex-cli/src/main.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,9 @@ struct InteractiveArgs {
319319
#[arg(long = "debug", help_heading = "Debugging")]
320320
debug: bool,
321321

322-
/// Initial prompt (if no subcommand)
323-
#[arg(trailing_var_arg = true)]
322+
/// Initial prompt (if no subcommand).
323+
/// Prompts starting with a dash (e.g., "--help explain this") are supported.
324+
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
324325
prompt: Vec<String>,
325326
}
326327

cortex-cli/src/mcp_cmd.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,14 @@ pub struct GetArgs {
305305
#[derive(Debug, Parser)]
306306
#[command(
307307
override_usage = "cortex mcp add [OPTIONS] <NAME> (--url <URL> | -- <COMMAND>...)",
308-
after_help = "Note: When adding a stdio server, use '--' before the command to separate it from options.\n\n\
308+
after_help = "IMPORTANT: Use '--' to separate the MCP server command from cortex options.\n\
309+
This is required when the command or its arguments start with a dash (-).\n\n\
309310
Examples:\n \
310311
cortex mcp add myserver -- npx @example/server\n \
311312
cortex mcp add myserver -- python -m my_server\n \
312-
cortex mcp add myserver --url https://example.com/mcp"
313+
cortex mcp add myserver -- node server.js -v # -v goes to server, not cortex\n \
314+
cortex mcp add myserver --url https://example.com/mcp\n\n\
315+
Without '--', arguments like '-v' or '-m' may be interpreted as cortex flags."
313316
)]
314317
pub struct AddArgs {
315318
/// Name for the MCP server configuration.
@@ -375,9 +378,10 @@ pub struct RemoveArgs {
375378
/// Name of the MCP server configuration to remove.
376379
pub name: String,
377380

378-
/// Force removal without confirmation.
379-
#[arg(short, long)]
380-
pub force: bool,
381+
/// Skip confirmation prompt.
382+
/// Aliases: --force, -f (for compatibility)
383+
#[arg(short = 'y', long = "yes", visible_aliases = ["force"], short_alias = 'f')]
384+
pub yes: bool,
381385
}
382386

383387
/// Auth command with subcommands.
@@ -926,7 +930,7 @@ url = "{url_escaped}"
926930
}
927931

928932
async fn run_remove(args: RemoveArgs) -> Result<()> {
929-
let RemoveArgs { name, force } = args;
933+
let RemoveArgs { name, yes } = args;
930934

931935
validate_server_name(&name)?;
932936

@@ -935,8 +939,8 @@ async fn run_remove(args: RemoveArgs) -> Result<()> {
935939
bail!("No MCP server named '{}' found", name);
936940
}
937941

938-
// Confirm removal if not forced
939-
if !force {
942+
// Confirm removal if not skipped via --yes/-y/--force/-f
943+
if !yes {
940944
print!("Remove MCP server '{name}'? [y/N]: ");
941945
io::stdout().flush()?;
942946

cortex-cli/src/run_cmd.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,13 @@ impl std::fmt::Display for OutputFormat {
6262

6363
/// Run CLI command for non-interactive execution.
6464
#[derive(Debug, Parser)]
65+
#[command(allow_hyphen_values = true)]
6566
pub struct RunCli {
6667
/// Message to send to the AI agent.
6768
/// Multiple arguments are joined with spaces.
68-
#[arg(trailing_var_arg = true)]
69+
/// Note: Prompts starting with a dash (e.g., "--help explain this flag")
70+
/// are supported and won't be interpreted as flags.
71+
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
6972
pub message: Vec<String>,
7073

7174
/// Execute a predefined command instead of a prompt.
@@ -167,6 +170,11 @@ pub struct RunCli {
167170
#[arg(long = "cwd", value_name = "DIR")]
168171
pub cwd: Option<PathBuf>,
169172

173+
/// Additional directories that should be writable.
174+
/// Can be specified multiple times to add multiple directories.
175+
#[arg(long = "add-dir", value_name = "DIR", action = clap::ArgAction::Append)]
176+
pub add_dir: Vec<PathBuf>,
177+
170178
/// Enable verbose/debug output.
171179
#[arg(long = "verbose", short = 'v')]
172180
pub verbose: bool,
@@ -887,6 +895,8 @@ impl RunCli {
887895

888896
// Output as a single JSON line (JSONL format)
889897
println!("{}", serde_json::to_string(&event_json)?);
898+
// Flush immediately for piped output to ensure streaming works
899+
io::stdout().flush()?;
890900
}
891901

892902
match &event.msg {

cortex-tui/src/modal/mod.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,16 @@ impl ModalStack {
219219
self.stack.last_mut()
220220
}
221221

222-
/// Handle a key event, delegating to the top modal
222+
/// Handle a key event, delegating to the top modal.
223+
///
224+
/// Key events are always sent to the topmost (most recently pushed) modal only.
225+
/// This ensures that nested modals (e.g., Config -> Privacy Settings) receive
226+
/// events in the correct order:
227+
/// - Pressing Escape closes the inner modal first
228+
/// - Only after the inner modal is closed does Escape reach the outer modal
229+
///
230+
/// This follows the standard z-order convention where the topmost UI element
231+
/// has input focus.
223232
pub fn handle_key(&mut self, key: KeyEvent) -> ModalResult {
224233
if let Some(modal) = self.current_mut() {
225234
let result = modal.handle_key(key);

cortex-tui/src/runner/event_loop.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,6 +1868,17 @@ impl EventLoop {
18681868
}
18691869
}
18701870

1871+
// Handle Copy action specially since it needs terminal access
1872+
// This handles both Ctrl+C (when text selected) and Ctrl+Shift+C
1873+
if action == KeyAction::Copy {
1874+
if self.app_state.text_selection.has_selection() {
1875+
self.copy_selection_to_clipboard(terminal)?;
1876+
self.app_state.text_selection.clear();
1877+
}
1878+
self.render(terminal)?;
1879+
return Ok(());
1880+
}
1881+
18711882
// Forward key events to input widget when focused and no action mapped
18721883
// This allows character input, backspace, delete, etc. to work
18731884
if context == ActionContext::Input && action == KeyAction::None {

cortex-update/src/download.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,36 @@ pub struct Downloader {
5858
}
5959

6060
impl Downloader {
61-
/// Create a new downloader.
61+
/// Create a new downloader with a secure temporary directory.
62+
///
63+
/// Uses a randomly-named subdirectory to prevent symlink attacks and
64+
/// predictable file name exploits.
6265
pub fn new(client: CortexSoftwareClient) -> UpdateResult<Self> {
63-
let temp_dir = std::env::temp_dir().join("cortex-update");
64-
std::fs::create_dir_all(&temp_dir)?;
66+
// Use a random suffix to prevent predictable temp directory names
67+
// This mitigates symlink attacks where an attacker pre-creates
68+
// files with expected names
69+
let random_suffix: u64 = std::time::SystemTime::now()
70+
.duration_since(std::time::UNIX_EPOCH)
71+
.map(|d| d.as_nanos() as u64)
72+
.unwrap_or(0)
73+
^ std::process::id() as u64;
74+
75+
let temp_dir = std::env::temp_dir().join(format!("cortex-update-{:x}", random_suffix));
76+
77+
// Create with restrictive permissions on Unix
78+
#[cfg(unix)]
79+
{
80+
use std::os::unix::fs::DirBuilderExt;
81+
std::fs::DirBuilder::new()
82+
.mode(0o700) // Owner-only access
83+
.recursive(true)
84+
.create(&temp_dir)?;
85+
}
86+
87+
#[cfg(not(unix))]
88+
{
89+
std::fs::create_dir_all(&temp_dir)?;
90+
}
6591

6692
Ok(Self { client, temp_dir })
6793
}

0 commit comments

Comments
 (0)