Skip to content

Commit dbce4fd

Browse files
committed
refactor(cortex-cli): eliminate code duplication using centralized utilities
- Add comprehensive terminal utilities in utils/terminal.rs: - Safe output functions (safe_print, safe_println, safe_eprint, safe_eprintln) that handle broken pipes gracefully - Centralized ToolDisplay struct and get_tool_display() function - Improved TermColor enum with stderr formatting support - Update utils/mod.rs with complete re-exports: - All terminal utilities (TermColor, ToolDisplay, colors_disabled, etc.) - Path utilities (is_sensitive_path, safe_join, SENSITIVE_PATHS) - Session utilities (get_most_recent_session) - Validation constants (ALLOWED_URL_SCHEMES, MAX_* limits, etc.) - Eliminate ~150 lines of duplicate code from run_cmd.rs: - Remove duplicate TermColor enum definition - Remove duplicate get_tool_display() function - Remove duplicate should_use_colors() function - Remove duplicate SENSITIVE_PATHS constant - Remove duplicate is_sensitive_path() function - Fix noop clone warning by using .to_string() instead This follows DRY principles by establishing utils/ as the single source of truth for common CLI functionality.
1 parent 0835958 commit dbce4fd

File tree

3 files changed

+218
-171
lines changed

3 files changed

+218
-171
lines changed

cortex-cli/src/run_cmd.rs

Lines changed: 5 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -261,106 +261,8 @@ pub struct RunCli {
261261
pub best_of: Option<u32>,
262262
}
263263

264-
/// Tool display information for formatted output.
265-
struct ToolDisplay {
266-
name: &'static str,
267-
color: TermColor,
268-
}
269-
270-
/// Terminal colors for output formatting.
271-
#[derive(Clone, Copy)]
272-
enum TermColor {
273-
Red,
274-
Green,
275-
Yellow,
276-
Blue,
277-
Magenta,
278-
Cyan,
279-
White,
280-
Default,
281-
}
282-
283-
impl TermColor {
284-
fn ansi_code(&self) -> &'static str {
285-
match self {
286-
TermColor::Red => "\x1b[1;31m",
287-
TermColor::Green => "\x1b[1;32m",
288-
TermColor::Yellow => "\x1b[1;33m",
289-
TermColor::Blue => "\x1b[1;34m",
290-
TermColor::Magenta => "\x1b[1;35m",
291-
TermColor::Cyan => "\x1b[1;36m",
292-
TermColor::White => "\x1b[1;37m",
293-
TermColor::Default => "\x1b[0m",
294-
}
295-
}
296-
297-
/// Get ANSI code only if colors should be shown (stdout is a TTY and NO_COLOR is not set).
298-
fn code_if_tty(&self) -> &'static str {
299-
if should_use_colors() {
300-
self.ansi_code()
301-
} else {
302-
""
303-
}
304-
}
305-
}
306-
307-
/// Check if colors should be used in output.
308-
/// Respects NO_COLOR environment variable and checks if stdout is a TTY.
309-
fn should_use_colors() -> bool {
310-
// Respect NO_COLOR environment variable (https://no-color.org/)
311-
if std::env::var("NO_COLOR").is_ok() {
312-
return false;
313-
}
314-
315-
// Check if stdout is a terminal
316-
io::stdout().is_terminal()
317-
}
318-
319-
/// Get tool display information.
320-
fn get_tool_display(tool_name: &str) -> ToolDisplay {
321-
match tool_name.to_lowercase().as_str() {
322-
"todowrite" | "todoread" => ToolDisplay {
323-
name: "Todo",
324-
color: TermColor::Yellow,
325-
},
326-
"bash" | "execute" => ToolDisplay {
327-
name: "Bash",
328-
color: TermColor::Red,
329-
},
330-
"edit" | "multiedit" => ToolDisplay {
331-
name: "Edit",
332-
color: TermColor::Green,
333-
},
334-
"glob" => ToolDisplay {
335-
name: "Glob",
336-
color: TermColor::Blue,
337-
},
338-
"grep" => ToolDisplay {
339-
name: "Grep",
340-
color: TermColor::Blue,
341-
},
342-
"list" | "ls" => ToolDisplay {
343-
name: "List",
344-
color: TermColor::Blue,
345-
},
346-
"read" => ToolDisplay {
347-
name: "Read",
348-
color: TermColor::Cyan,
349-
},
350-
"write" | "create" => ToolDisplay {
351-
name: "Write",
352-
color: TermColor::Green,
353-
},
354-
"websearch" | "fetchurl" => ToolDisplay {
355-
name: "Search",
356-
color: TermColor::Magenta,
357-
},
358-
_ => ToolDisplay {
359-
name: "Tool",
360-
color: TermColor::White,
361-
},
362-
}
363-
}
264+
// Re-use centralized terminal utilities from utils module
265+
use crate::utils::{TermColor, get_tool_display};
364266

365267
/// File attachment information.
366268
#[derive(Debug, Clone, Serialize)]
@@ -550,62 +452,8 @@ impl RunCli {
550452
}
551453
}
552454

553-
/// Sensitive system paths that should trigger a security warning.
554-
/// These files contain sensitive information that users may not want to send to AI providers.
555-
const SENSITIVE_PATHS: &[&str] = &[
556-
"/etc/passwd",
557-
"/etc/shadow",
558-
"/etc/group",
559-
"/etc/gshadow",
560-
"/etc/sudoers",
561-
"/etc/ssh/",
562-
"/etc/ssl/",
563-
"/etc/pki/",
564-
"/.ssh/",
565-
"/.gnupg/",
566-
"/.aws/",
567-
"/.azure/",
568-
"/.gcloud/",
569-
"/.kube/",
570-
"/.docker/config.json",
571-
"/.npmrc",
572-
"/.pypirc",
573-
"/.netrc",
574-
"/.gitconfig",
575-
"/id_rsa",
576-
"/id_dsa",
577-
"/id_ecdsa",
578-
"/id_ed25519",
579-
"/.env",
580-
"/credentials",
581-
"/secrets",
582-
"/private",
583-
"/token",
584-
];
585-
586-
/// Check if a path appears to be a sensitive system file.
587-
fn is_sensitive_path(path: &Path) -> bool {
588-
let path_str = path.to_string_lossy().to_lowercase();
589-
590-
for sensitive in SENSITIVE_PATHS {
591-
if path_str.contains(&sensitive.to_lowercase()) {
592-
return true;
593-
}
594-
}
595-
596-
// Also check for common sensitive file extensions
597-
if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
598-
let ext_lower = ext.to_lowercase();
599-
if matches!(
600-
ext_lower.as_str(),
601-
"pem" | "key" | "crt" | "cer" | "pfx" | "p12"
602-
) {
603-
return true;
604-
}
605-
}
606-
607-
false
608-
}
455+
// Use centralized sensitive path detection from utils module
456+
use crate::utils::is_sensitive_path;
609457

610458
/// Validate that a file is safe to attach (not a device file, not too large).
611459
/// Also warns about sensitive system files that may contain private information.
@@ -936,7 +784,7 @@ impl RunCli {
936784
// Try to get the custom command registry
937785
if let Some(registry) = cortex_engine::try_custom_command_registry() {
938786
// Try to execute the custom command
939-
let ctx = cortex_engine::TemplateContext::new(message.clone()).with_cwd(
787+
let ctx = cortex_engine::TemplateContext::new(message.to_string()).with_cwd(
940788
std::env::current_dir()
941789
.unwrap_or_default()
942790
.to_string_lossy()

cortex-cli/src/utils/mod.rs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,43 @@
66
//! - File validation and security checks
77
//! - Clipboard operations
88
//! - Path validation utilities
9-
//! - Terminal color detection
9+
//! - Terminal color detection and safe output
10+
//!
11+
//! # Design Principles
12+
//!
13+
//! - **DRY (Don't Repeat Yourself)**: All common functionality is centralized here
14+
//! - **Safety**: Output functions handle broken pipes gracefully
15+
//! - **Consistency**: Validation rules are uniform across all commands
1016
1117
pub mod clipboard;
1218
pub mod paths;
1319
pub mod session;
1420
pub mod terminal;
1521
pub mod validation;
1622

23+
// Re-export clipboard operations
1724
pub use clipboard::{copy_to_clipboard, read_clipboard};
18-
pub use paths::{expand_tilde, get_cortex_home, validate_path_safety};
19-
pub use session::{SessionIdError, resolve_session_id};
20-
pub use terminal::{TermColor, is_terminal_output, should_use_colors};
25+
26+
// Re-export path utilities
27+
pub use paths::{
28+
SENSITIVE_PATHS, expand_tilde, get_cortex_home, is_sensitive_path, safe_join,
29+
validate_path_safety,
30+
};
31+
32+
// Re-export session utilities
33+
pub use session::{SessionIdError, get_most_recent_session, resolve_session_id};
34+
35+
// Re-export terminal utilities
36+
pub use terminal::{
37+
TermColor, ToolDisplay, colors_disabled, format_duration, format_size, get_tool_display,
38+
is_light_theme, is_terminal_output, restore_terminal, safe_eprint, safe_eprintln, safe_print,
39+
safe_println, safe_println_empty, should_use_colors, should_use_colors_stderr,
40+
};
41+
42+
// Re-export validation utilities
2143
pub use validation::{
44+
ALLOWED_URL_SCHEMES, BLOCKED_URL_PATTERNS, MAX_ENV_VAR_NAME_LENGTH, MAX_ENV_VAR_VALUE_LENGTH,
45+
MAX_SERVER_NAME_LENGTH, MAX_URL_LENGTH, RESERVED_COMMAND_NAMES, is_reserved_command,
2246
validate_env_var_name, validate_env_var_value, validate_model_name, validate_server_name,
2347
validate_url, validate_url_allowing_local,
2448
};

0 commit comments

Comments
 (0)