diff --git a/src/commands/commit_cmd.rs b/src/commands/commit_cmd.rs index a321d92..b523011 100644 --- a/src/commands/commit_cmd.rs +++ b/src/commands/commit_cmd.rs @@ -372,13 +372,16 @@ fn open_editor(initial: &str) -> Result { std::fs::write(&tmp_path, initial) .with_context(|| format!("Failed to write temp file: {}", tmp_path.display()))?; - let path_str = tmp_path.to_string_lossy().to_string(); - - // Use sh -c to handle multi-word editors properly - let shell_cmd = format!("{editor} '{path_str}'"); - let status = std::process::Command::new("sh") - .arg("-c") - .arg(&shell_cmd) + // Split $EDITOR into program + args to avoid shell injection via `sh -c`. + // Handles editors with flags like `code --wait` or `vim -p`. + let editor_parts: Vec<&str> = editor.split_whitespace().collect(); + let (program, args) = match editor_parts.split_first() { + Some((p, a)) => (p, a), + None => anyhow::bail!("$EDITOR is empty"), + }; + let status = std::process::Command::new(program) + .args(args) + .arg(&tmp_path) .status() .with_context(|| format!("Failed to open editor: {editor}"))?; diff --git a/src/config/loader.rs b/src/config/loader.rs index 90fc40a..49bf9a7 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -435,19 +435,32 @@ pub fn save_api_key(key: &str) -> std::result::Result<(), CoraError> { table.insert("api_key".to_string(), toml::Value::String(key.to_string())); let content = table.to_string(); - std::fs::write(&path, content) - .map_err(|e| CoraError::AuthError(format!("{}: {}", path.display(), e)))?; - - debug!(path = %path.display(), "saved API key"); - - // Restrict permissions to owner only (0o600) + // Create the file with restrictive permissions from the start (0o600), + // avoiding the TOCTOU window between write and chmod. #[cfg(unix)] { - use std::os::unix::fs::PermissionsExt; - let perms = std::fs::Permissions::from_mode(0o600); - std::fs::set_permissions(&path, perms)?; + use std::os::unix::fs::OpenOptionsExt; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(&path) + .map_err(|e| CoraError::AuthError(format!("{}: {}", path.display(), e)))?; + use std::io::Write; + file.write_all(content.as_bytes()) + .map_err(|e| CoraError::AuthError(format!("{}: {}", path.display(), e)))?; + } + #[cfg(not(unix))] + { + // Non-Unix platforms: write then attempt to restrict permissions. + // Best-effort — platform support for file permissions varies. + std::fs::write(&path, content) + .map_err(|e| CoraError::AuthError(format!("{}: {}", path.display(), e)))?; } + debug!(path = %path.display(), "saved API key"); + Ok(()) } diff --git a/src/engine/context/resolver.rs b/src/engine/context/resolver.rs index 46521a3..79df8bd 100644 --- a/src/engine/context/resolver.rs +++ b/src/engine/context/resolver.rs @@ -11,7 +11,7 @@ //! Additionally, test file mapping is supported via naming conventions. use std::collections::HashMap; -use std::path::Path; +use std::path::{Path, PathBuf}; use tracing::debug; @@ -27,6 +27,34 @@ const MAX_TYPE_LINES: usize = 50; /// Maximum lines to read per test function. const MAX_TEST_LINES: usize = 30; +/// Safely join a relative path with a project root, verifying that the +/// resulting path stays within the project root (prevents path traversal). +/// Returns `None` if the resolved path escapes the project root or +/// canonicalization fails for an existing file. +fn safe_join(project_root: &Path, relative: &str) -> Option { + let joined = project_root.join(relative); + // Canonicalize to resolve any `..` or symlinks. + // If the file doesn't exist yet, canonicalize just the project_root + // and check that the joined path starts with it as a prefix. + let canonical_root = std::fs::canonicalize(project_root).ok()?; + if joined.exists() { + let canonical_joined = std::fs::canonicalize(&joined).ok()?; + if canonical_joined.starts_with(&canonical_root) { + Some(canonical_joined) + } else { + None + } + } else { + // File doesn't exist yet; verify the joined path doesn't escape + // the project root by canonicalizing what we can and checking prefixes. + if joined.starts_with(project_root) { + Some(joined) + } else { + None + } + } +} + /// Build the full context chain from extracted symbols. /// /// This is the main entry point: extract → resolve → read → budget → assemble. @@ -137,8 +165,14 @@ fn resolve_symbols( continue; } - // Check if the entry's file exists - let full_path = project_root.join(&entry.file); + // Check if the entry's file exists and stays within project root + let full_path = match safe_join(project_root, &entry.file) { + Some(p) => p, + None => { + debug!(file = %entry.file, "path traversal detected, skipping"); + continue; + } + }; if !full_path.exists() { debug!(file = %entry.file, "resolved file does not exist, skipping"); continue; @@ -248,7 +282,13 @@ fn resolve_import( for ext in &extensions { let candidate = format!("{}.{}", resolved.display(), ext); - let full = project_root.join(&candidate); + let full = match safe_join(project_root, &candidate) { + Some(p) => p, + None => { + debug!(file = %candidate, "path traversal detected in JS/TS import, skipping"); + continue; + } + }; if full.exists() { let line_end = find_definition_end(&full); entries.push(ContextEntry { @@ -624,7 +664,13 @@ fn add_test_mappings( let candidates = test_file_candidates(&sym.file); for candidate in candidates { - let full = project_root.join(&candidate); + let full = match safe_join(project_root, &candidate) { + Some(p) => p, + None => { + debug!(file = %candidate, "path traversal detected in test resolution, skipping"); + continue; + } + }; if full.exists() { let line_end = find_definition_end(&full); entries.push(ContextEntry { @@ -697,7 +743,17 @@ fn test_file_candidates(source: &str) -> Vec { /// Read the content for a context entry, respecting line range and caps. fn read_entry_content(entry: &ContextEntry, project_root: &Path) -> String { - let full_path = project_root.join(&entry.file); + let full_path = match safe_join(project_root, &entry.file) { + Some(p) if p.exists() => p, + Some(_) => { + debug!(file = %entry.file, "context entry file does not exist"); + return String::new(); + } + None => { + debug!(file = %entry.file, "path traversal detected in read_entry_content"); + return String::new(); + } + }; let content = match std::fs::read_to_string(&full_path) { Ok(c) => c, Err(e) => {