Skip to content

Commit 67d2bad

Browse files
authored
fix(builtins): reject path traversal in patch diff headers (#1051)
## Summary - Add `validate_path()` that rejects paths containing `..` components after `strip_path()` - Prevents crafted diff headers like `--- a/../../../etc/passwd` from writing outside working directory - Normal patch paths still work correctly ## Test plan - [ ] `../../../etc/passwd` traversal rejected with error - [ ] `foo/../../secret.txt` embedded traversal rejected - [ ] Normal clean paths after strip still work - [ ] All 20 patch unit tests pass - [ ] All existing spec tests pass Closes #989
1 parent 2e8039c commit 67d2bad

File tree

1 file changed

+90
-1
lines changed

1 file changed

+90
-1
lines changed

crates/bashkit/src/builtins/patch.rs

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,20 @@ fn strip_path(path: &str, strip: usize) -> String {
9090
}
9191
}
9292

93+
/// Validate that a path does not escape the working directory via `..` components.
94+
/// Rejects any path containing a literal `..` path segment to prevent directory traversal.
95+
fn validate_path(path: &str) -> std::result::Result<(), String> {
96+
for component in path.split('/') {
97+
if component == ".." {
98+
return Err(format!(
99+
"patch: rejecting path '{}': contains '..' traversal",
100+
path
101+
));
102+
}
103+
}
104+
Ok(())
105+
}
106+
93107
/// Parse unified diff text into file diffs
94108
fn parse_unified_diff(input: &str) -> Vec<FileDiff> {
95109
let mut diffs = Vec::new();
@@ -314,7 +328,13 @@ impl Builtin for Patch {
314328
&diff.new_path
315329
}
316330
};
317-
strip_path(raw_path, opts.strip)
331+
let stripped = strip_path(raw_path, opts.strip);
332+
if let Err(e) = validate_path(&stripped) {
333+
output.push_str(&format!("{}\n", e));
334+
had_error = true;
335+
continue;
336+
}
337+
stripped
318338
};
319339

320340
let path = resolve_path(ctx.cwd, &target);
@@ -608,6 +628,75 @@ mod tests {
608628
assert_eq!(hunk.new_count, 2);
609629
}
610630

631+
// --- Security tests for path traversal (issue #989) ---
632+
633+
#[tokio::test]
634+
async fn test_patch_rejects_path_traversal() {
635+
let diff = "\
636+
--- a/../../../etc/passwd
637+
+++ b/../../../etc/passwd
638+
@@ -1,1 +1,1 @@
639+
-root:x:0:0:root:/root:/bin/bash
640+
+pwned
641+
";
642+
let (result, _fs) = run_patch(&["-p1"], diff, &[]).await;
643+
assert_eq!(result.exit_code, 1);
644+
assert!(
645+
result.stderr.contains(".."),
646+
"error should mention path traversal: {}",
647+
result.stderr
648+
);
649+
}
650+
651+
#[tokio::test]
652+
async fn test_patch_rejects_embedded_dotdot() {
653+
let diff = "\
654+
--- a/foo/../../secret.txt
655+
+++ b/foo/../../secret.txt
656+
@@ -1,1 +1,1 @@
657+
-old
658+
+new
659+
";
660+
let (result, _fs) = run_patch(&["-p1"], diff, &[("/secret.txt", b"old\n")]).await;
661+
assert_eq!(result.exit_code, 1);
662+
assert!(result.stderr.contains(".."));
663+
}
664+
665+
#[tokio::test]
666+
async fn test_patch_allows_clean_path_after_strip() {
667+
let diff = "\
668+
--- a/main.rs
669+
+++ b/main.rs
670+
@@ -1,1 +1,1 @@
671+
-old
672+
+new
673+
";
674+
let (result, _fs) = run_patch(&["-p1"], diff, &[("/main.rs", b"old\n")]).await;
675+
assert_eq!(result.exit_code, 0);
676+
}
677+
678+
#[tokio::test]
679+
async fn test_strip_path_preserves_dotdot() {
680+
assert_eq!(
681+
strip_path("a/../../../etc/passwd", 1),
682+
"../../../etc/passwd"
683+
);
684+
}
685+
686+
#[tokio::test]
687+
async fn test_validate_path_rejects_dotdot() {
688+
assert!(validate_path("../../../etc/passwd").is_err());
689+
assert!(validate_path("foo/../../bar").is_err());
690+
assert!(validate_path("..").is_err());
691+
}
692+
693+
#[tokio::test]
694+
async fn test_validate_path_allows_clean() {
695+
assert!(validate_path("foo/bar/baz.txt").is_ok());
696+
assert!(validate_path("file.txt").is_ok());
697+
assert!(validate_path("a/b/c").is_ok());
698+
}
699+
611700
#[tokio::test]
612701
async fn test_patch_delete_file_removes_from_vfs() {
613702
// Create a file, then apply a delete patch (new_path = /dev/null)

0 commit comments

Comments
 (0)