Skip to content

Commit 07906af

Browse files
committed
fix(common): improve path validation and tilde expansion
- Fix #5181: validate_path_safety() now checks path components instead of substring match, preventing false positives for filenames like 'file..txt' - Fix #5183: expand_tilde() now handles bare '~' path in addition to '~/' prefix - Updated tests to verify correct behavior
1 parent 3ebcd39 commit 07906af

1 file changed

Lines changed: 46 additions & 19 deletions

File tree

src/cortex-cli/src/utils/paths.rs

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,16 @@ pub fn get_cortex_home() -> PathBuf {
3434
/// // Returns: /home/user/documents/file.txt
3535
/// ```
3636
pub fn expand_tilde(path: &str) -> String {
37-
if path.starts_with("~/")
38-
&& let Some(home) = dirs::home_dir()
39-
{
40-
return home.join(&path[2..]).to_string_lossy().to_string();
37+
if path == "~" {
38+
// Handle bare "~" - return home directory
39+
if let Some(home) = dirs::home_dir() {
40+
return home.to_string_lossy().to_string();
41+
}
42+
} else if path.starts_with("~/") {
43+
// Handle "~/" prefix - expand to home directory + rest of path
44+
if let Some(home) = dirs::home_dir() {
45+
return home.join(&path[2..]).to_string_lossy().to_string();
46+
}
4147
}
4248
path.to_string()
4349
}
@@ -58,8 +64,12 @@ pub fn expand_tilde(path: &str) -> String {
5864
pub fn validate_path_safety(path: &Path, base_dir: Option<&Path>) -> Result<(), String> {
5965
let path_str = path.to_string_lossy();
6066

61-
// Check for path traversal attempts
62-
if path_str.contains("..") {
67+
// Check for path traversal attempts by examining path components
68+
// This correctly handles filenames containing ".." like "file..txt"
69+
if path
70+
.components()
71+
.any(|c| matches!(c, std::path::Component::ParentDir))
72+
{
6373
return Err("Path contains traversal sequence '..'".to_string());
6474
}
6575

@@ -257,8 +267,15 @@ mod tests {
257267

258268
#[test]
259269
fn test_expand_tilde_with_tilde_only() {
260-
// Test tilde alone - should remain unchanged (not "~/")
261-
assert_eq!(expand_tilde("~"), "~");
270+
// Test bare "~" - should expand to home directory
271+
let result = expand_tilde("~");
272+
if let Some(home) = dirs::home_dir() {
273+
let expected = home.to_string_lossy().to_string();
274+
assert_eq!(result, expected);
275+
} else {
276+
// If no home dir, original is returned
277+
assert_eq!(result, "~");
278+
}
262279
}
263280

264281
#[test]
@@ -320,20 +337,30 @@ mod tests {
320337

321338
#[test]
322339
fn test_validate_path_safety_detects_various_traversal_patterns() {
323-
// Different traversal patterns
324-
let patterns = ["foo/../bar", "...", "foo/bar/../baz", "./foo/../../../etc"];
340+
// Patterns that ARE path traversal (contain ".." as a component)
341+
let traversal_patterns = ["foo/../bar", "foo/bar/../baz", "./foo/../../../etc", ".."];
325342

326-
for pattern in patterns {
343+
for pattern in traversal_patterns {
327344
let path = Path::new(pattern);
328345
let result = validate_path_safety(path, None);
329-
// Only patterns containing ".." should fail
330-
if pattern.contains("..") {
331-
assert!(
332-
result.is_err(),
333-
"Expected traversal detection for: {}",
334-
pattern
335-
);
336-
}
346+
assert!(
347+
result.is_err(),
348+
"Expected traversal detection for: {}",
349+
pattern
350+
);
351+
}
352+
353+
// Patterns that are NOT path traversal (contain ".." in filenames only)
354+
let safe_patterns = ["file..txt", "..hidden", "test...file", "foo/bar..baz/file"];
355+
356+
for pattern in safe_patterns {
357+
let path = Path::new(pattern);
358+
let result = validate_path_safety(path, None);
359+
assert!(
360+
result.is_ok(),
361+
"False positive: '{}' should not be detected as traversal",
362+
pattern
363+
);
337364
}
338365
}
339366

0 commit comments

Comments
 (0)