From 7055c0806841884d947c9d3558fd1879764fd37e Mon Sep 17 00:00:00 2001 From: juliuss007 <157698750+juliuss007@users.noreply.github.com> Date: Tue, 26 May 2026 18:48:30 +0200 Subject: [PATCH] fix: reject absolute apply patch paths --- src/cortex-engine/src/agent/tools/patch.rs | 42 ++++++++++++++++++- .../src/tools/handlers/apply_patch.rs | 37 ++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/cortex-engine/src/agent/tools/patch.rs b/src/cortex-engine/src/agent/tools/patch.rs index e9142ed6f..f7de01db9 100644 --- a/src/cortex-engine/src/agent/tools/patch.rs +++ b/src/cortex-engine/src/agent/tools/patch.rs @@ -124,7 +124,7 @@ impl PatchTool { if change.is_deleted && let Some(ref old_path) = change.old_path { - let full_path = cwd.join(old_path); + let full_path = resolve_patch_path(cwd, old_path)?; if !dry_run { if full_path.exists() { fs::remove_file(&full_path) @@ -142,7 +142,7 @@ impl PatchTool { .or(change.old_path.as_ref()) .ok_or_else(|| "No file path specified in diff".to_string())?; - let full_path = cwd.join(target_path); + let full_path = resolve_patch_path(cwd, target_path)?; // Handle new file if change.is_new_file { @@ -332,6 +332,18 @@ fn parse_diff_path(path_str: &str) -> Option { } } +fn resolve_patch_path(cwd: &Path, path: &Path) -> std::result::Result { + if path.is_absolute() { + return Err(format!( + "Absolute patch paths are not allowed: {}", + path.display() + )); + } + + crate::security::path_safety::resolve_and_validate_path(path, cwd, cwd) + .map_err(|e| format!("Patch path escapes workspace: {e}")) +} + fn parse_hunk_header(line: &str) -> Option { // @@ -1,5 +1,6 @@ let line = line.trim_start_matches("@@").trim_end_matches("@@").trim(); @@ -479,3 +491,29 @@ fn matches_at_position(lines: &[String], match_lines: &[&str], start: usize) -> } true } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn apply_patch_rejects_absolute_new_file_path() { + let temp_dir = tempfile::tempdir().unwrap(); + let absolute_path = temp_dir.path().join("escaped.txt"); + let patch = format!( + "--- /dev/null\n+++ {}\n@@ -0,0 +1 @@\n+owned\n", + absolute_path.display() + ); + + let result = PatchTool::new() + .apply_patch(&patch, temp_dir.path(), false) + .await; + + assert!( + result + .unwrap_err() + .contains("Absolute patch paths are not allowed") + ); + assert!(!absolute_path.exists()); + } +} diff --git a/src/cortex-engine/src/tools/handlers/apply_patch.rs b/src/cortex-engine/src/tools/handlers/apply_patch.rs index 65ade88f9..836e7f66c 100644 --- a/src/cortex-engine/src/tools/handlers/apply_patch.rs +++ b/src/cortex-engine/src/tools/handlers/apply_patch.rs @@ -3,7 +3,7 @@ //! Complete unified diff parser and applier with context matching. use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use async_trait::async_trait; use serde::Deserialize; @@ -226,6 +226,18 @@ fn parse_file_path(path_str: &str) -> Option { } } +fn resolve_patch_path(cwd: &Path, path: &Path) -> std::result::Result { + if path.is_absolute() { + return Err(format!( + "Absolute patch paths are not allowed: {}", + path.display() + )); + } + + crate::security::path_safety::resolve_and_validate_path(path, cwd, cwd) + .map_err(|e| format!("Patch path escapes workspace: {e}")) +} + /// Parse a hunk header like "@@ -1,5 +1,6 @@". fn parse_hunk_header(line: &str) -> Option { let line = line.trim_start_matches('@').trim_end_matches('@').trim(); @@ -265,7 +277,7 @@ fn apply_file_change( if change.is_deleted && let Some(ref old_path) = change.old_path { - let full_path = cwd.join(old_path); + let full_path = resolve_patch_path(cwd, old_path)?; if !dry_run { fs::remove_file(&full_path) .map_err(|e| format!("Failed to delete {}: {}", full_path.display(), e))?; @@ -280,7 +292,7 @@ fn apply_file_change( .or(change.old_path.as_ref()) .ok_or_else(|| "No file path specified".to_string())?; - let full_path = cwd.join(target_path); + let full_path = resolve_patch_path(cwd, target_path)?; // Handle new file if change.is_new_file { @@ -508,4 +520,23 @@ mod tests { let changes = parse_unified_diff(patch).unwrap(); assert!(changes[0].is_new_file); } + + #[test] + fn test_apply_patch_rejects_absolute_new_file_path() { + let temp_dir = tempfile::tempdir().unwrap(); + let absolute_path = temp_dir.path().join("escaped.txt"); + let patch = format!( + "--- /dev/null\n+++ {}\n@@ -0,0 +1 @@\n+owned\n", + absolute_path.display() + ); + + let result = apply_unified_diff(&patch, &temp_dir.path().to_path_buf(), false); + + assert!( + result + .unwrap_err() + .contains("Absolute patch paths are not allowed") + ); + assert!(!absolute_path.exists()); + } }