Skip to content

Commit 7055c08

Browse files
committed
fix: reject absolute apply patch paths
1 parent 7954d02 commit 7055c08

2 files changed

Lines changed: 74 additions & 5 deletions

File tree

src/cortex-engine/src/agent/tools/patch.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ impl PatchTool {
124124
if change.is_deleted
125125
&& let Some(ref old_path) = change.old_path
126126
{
127-
let full_path = cwd.join(old_path);
127+
let full_path = resolve_patch_path(cwd, old_path)?;
128128
if !dry_run {
129129
if full_path.exists() {
130130
fs::remove_file(&full_path)
@@ -142,7 +142,7 @@ impl PatchTool {
142142
.or(change.old_path.as_ref())
143143
.ok_or_else(|| "No file path specified in diff".to_string())?;
144144

145-
let full_path = cwd.join(target_path);
145+
let full_path = resolve_patch_path(cwd, target_path)?;
146146

147147
// Handle new file
148148
if change.is_new_file {
@@ -332,6 +332,18 @@ fn parse_diff_path(path_str: &str) -> Option<PathBuf> {
332332
}
333333
}
334334

335+
fn resolve_patch_path(cwd: &Path, path: &Path) -> std::result::Result<PathBuf, String> {
336+
if path.is_absolute() {
337+
return Err(format!(
338+
"Absolute patch paths are not allowed: {}",
339+
path.display()
340+
));
341+
}
342+
343+
crate::security::path_safety::resolve_and_validate_path(path, cwd, cwd)
344+
.map_err(|e| format!("Patch path escapes workspace: {e}"))
345+
}
346+
335347
fn parse_hunk_header(line: &str) -> Option<Hunk> {
336348
// @@ -1,5 +1,6 @@
337349
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) ->
479491
}
480492
true
481493
}
494+
495+
#[cfg(test)]
496+
mod tests {
497+
use super::*;
498+
499+
#[tokio::test]
500+
async fn apply_patch_rejects_absolute_new_file_path() {
501+
let temp_dir = tempfile::tempdir().unwrap();
502+
let absolute_path = temp_dir.path().join("escaped.txt");
503+
let patch = format!(
504+
"--- /dev/null\n+++ {}\n@@ -0,0 +1 @@\n+owned\n",
505+
absolute_path.display()
506+
);
507+
508+
let result = PatchTool::new()
509+
.apply_patch(&patch, temp_dir.path(), false)
510+
.await;
511+
512+
assert!(
513+
result
514+
.unwrap_err()
515+
.contains("Absolute patch paths are not allowed")
516+
);
517+
assert!(!absolute_path.exists());
518+
}
519+
}

src/cortex-engine/src/tools/handlers/apply_patch.rs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! Complete unified diff parser and applier with context matching.
44
55
use std::fs;
6-
use std::path::PathBuf;
6+
use std::path::{Path, PathBuf};
77

88
use async_trait::async_trait;
99
use serde::Deserialize;
@@ -226,6 +226,18 @@ fn parse_file_path(path_str: &str) -> Option<PathBuf> {
226226
}
227227
}
228228

229+
fn resolve_patch_path(cwd: &Path, path: &Path) -> std::result::Result<PathBuf, String> {
230+
if path.is_absolute() {
231+
return Err(format!(
232+
"Absolute patch paths are not allowed: {}",
233+
path.display()
234+
));
235+
}
236+
237+
crate::security::path_safety::resolve_and_validate_path(path, cwd, cwd)
238+
.map_err(|e| format!("Patch path escapes workspace: {e}"))
239+
}
240+
229241
/// Parse a hunk header like "@@ -1,5 +1,6 @@".
230242
fn parse_hunk_header(line: &str) -> Option<Hunk> {
231243
let line = line.trim_start_matches('@').trim_end_matches('@').trim();
@@ -265,7 +277,7 @@ fn apply_file_change(
265277
if change.is_deleted
266278
&& let Some(ref old_path) = change.old_path
267279
{
268-
let full_path = cwd.join(old_path);
280+
let full_path = resolve_patch_path(cwd, old_path)?;
269281
if !dry_run {
270282
fs::remove_file(&full_path)
271283
.map_err(|e| format!("Failed to delete {}: {}", full_path.display(), e))?;
@@ -280,7 +292,7 @@ fn apply_file_change(
280292
.or(change.old_path.as_ref())
281293
.ok_or_else(|| "No file path specified".to_string())?;
282294

283-
let full_path = cwd.join(target_path);
295+
let full_path = resolve_patch_path(cwd, target_path)?;
284296

285297
// Handle new file
286298
if change.is_new_file {
@@ -508,4 +520,23 @@ mod tests {
508520
let changes = parse_unified_diff(patch).unwrap();
509521
assert!(changes[0].is_new_file);
510522
}
523+
524+
#[test]
525+
fn test_apply_patch_rejects_absolute_new_file_path() {
526+
let temp_dir = tempfile::tempdir().unwrap();
527+
let absolute_path = temp_dir.path().join("escaped.txt");
528+
let patch = format!(
529+
"--- /dev/null\n+++ {}\n@@ -0,0 +1 @@\n+owned\n",
530+
absolute_path.display()
531+
);
532+
533+
let result = apply_unified_diff(&patch, &temp_dir.path().to_path_buf(), false);
534+
535+
assert!(
536+
result
537+
.unwrap_err()
538+
.contains("Absolute patch paths are not allowed")
539+
);
540+
assert!(!absolute_path.exists());
541+
}
511542
}

0 commit comments

Comments
 (0)