fix(security): avoid following symlinked workspace walks
This commit is contained in:
@@ -124,7 +124,7 @@ fn search_files(
|
||||
let mut results: Vec<FileSearchMatch> = Vec::new();
|
||||
|
||||
let mut builder = WalkBuilder::new(base_path);
|
||||
builder.hidden(false).follow_links(true).require_git(false);
|
||||
builder.hidden(false).follow_links(false).require_git(false);
|
||||
let walker = builder.build();
|
||||
|
||||
for entry in walker {
|
||||
@@ -322,4 +322,27 @@ mod tests {
|
||||
assert!(result.content.contains("main.rs"));
|
||||
assert!(!result.content.contains("notes.md"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn test_file_search_does_not_follow_symlinked_files() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
let outside = tmp.path().join("outside");
|
||||
std::fs::create_dir_all(&root).expect("mkdir workspace");
|
||||
std::fs::create_dir_all(&outside).expect("mkdir outside");
|
||||
let outside_file = outside.join("secret.txt");
|
||||
std::fs::write(&outside_file, "outside\n").expect("write outside");
|
||||
std::os::unix::fs::symlink(&outside_file, root.join("secret.txt")).expect("symlink");
|
||||
|
||||
let ctx = ToolContext::new(root);
|
||||
let tool = FileSearchTool;
|
||||
let result = tool
|
||||
.execute(json!({"query": "secret"}), &ctx)
|
||||
.await
|
||||
.expect("execute");
|
||||
|
||||
assert!(result.success);
|
||||
assert!(!result.content.contains("secret.txt"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -63,10 +63,13 @@ fn generate_project_map(root: &std::path::Path, max_depth: usize) -> Result<Proj
|
||||
// For key_files, we can just do a quick scan since summarize_project doesn't return them directly anymore
|
||||
let mut key_files = Vec::new();
|
||||
let mut builder = ignore::WalkBuilder::new(root);
|
||||
builder.hidden(false).follow_links(true).max_depth(Some(2));
|
||||
builder.hidden(false).follow_links(false).max_depth(Some(2));
|
||||
let walker = builder.build();
|
||||
|
||||
for entry in walker.flatten() {
|
||||
if entry.file_type().is_some_and(|ft| ft.is_symlink()) {
|
||||
continue;
|
||||
}
|
||||
if is_key_file(entry.path())
|
||||
&& let Ok(rel) = entry.path().strip_prefix(root)
|
||||
{
|
||||
|
||||
@@ -260,6 +260,16 @@ fn collect_files_recursive(
|
||||
for entry in entries {
|
||||
let entry = entry.map_err(|e| ToolError::execution_failed(e.to_string()))?;
|
||||
let path = entry.path();
|
||||
let file_type = entry.file_type().map_err(|e| {
|
||||
ToolError::execution_failed(format!(
|
||||
"Failed to inspect file type for {}: {}",
|
||||
path.display(),
|
||||
e
|
||||
))
|
||||
})?;
|
||||
if file_type.is_symlink() {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Get relative path for pattern matching
|
||||
let relative = path.strip_prefix(root).unwrap_or(&path);
|
||||
@@ -270,9 +280,9 @@ fn collect_files_recursive(
|
||||
continue;
|
||||
}
|
||||
|
||||
if path.is_dir() {
|
||||
if file_type.is_dir() {
|
||||
collect_files_recursive(root, &path, include_patterns, exclude_patterns, files)?;
|
||||
} else if path.is_file() {
|
||||
} else if file_type.is_file() {
|
||||
// Check inclusions (if any specified)
|
||||
if include_patterns.is_empty() || should_include(&relative_str, include_patterns) {
|
||||
files.push(path);
|
||||
@@ -544,6 +554,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn test_grep_files_does_not_follow_symlinked_files() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
let outside = tmp.path().join("outside");
|
||||
std::fs::create_dir_all(&root).expect("mkdir workspace");
|
||||
std::fs::create_dir_all(&outside).expect("mkdir outside");
|
||||
let outside_file = outside.join("secret.txt");
|
||||
fs::write(&outside_file, "NEEDLE\n").expect("write outside");
|
||||
std::os::unix::fs::symlink(&outside_file, root.join("secret.txt")).expect("symlink");
|
||||
|
||||
let ctx = ToolContext::new(root);
|
||||
let tool = GrepFilesTool;
|
||||
let result = tool
|
||||
.execute(json!({"pattern": "NEEDLE"}), &ctx)
|
||||
.await
|
||||
.expect("execute");
|
||||
|
||||
assert!(result.success);
|
||||
let parsed: Value = serde_json::from_str(&result.content).unwrap();
|
||||
assert_eq!(parsed["total_matches"].as_u64().unwrap(), 0);
|
||||
assert_eq!(parsed["files_searched"].as_u64().unwrap(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_grep_files_invalid_regex() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
+24
-2
@@ -51,7 +51,7 @@ pub fn summarize_project(root: &Path) -> String {
|
||||
let mut key_files = Vec::new();
|
||||
|
||||
let mut builder = WalkBuilder::new(root);
|
||||
builder.hidden(false).follow_links(true).max_depth(Some(2));
|
||||
builder.hidden(false).follow_links(false).max_depth(Some(2));
|
||||
let walker = builder.build();
|
||||
|
||||
for entry in walker {
|
||||
@@ -59,6 +59,9 @@ pub fn summarize_project(root: &Path) -> String {
|
||||
Ok(entry) => entry,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if entry.file_type().is_some_and(|ft| ft.is_symlink()) {
|
||||
continue;
|
||||
}
|
||||
if is_key_file(entry.path())
|
||||
&& let Ok(rel) = entry.path().strip_prefix(root)
|
||||
{
|
||||
@@ -113,10 +116,13 @@ pub fn project_tree(root: &Path, max_depth: usize) -> String {
|
||||
let mut builder = WalkBuilder::new(root);
|
||||
builder
|
||||
.hidden(false)
|
||||
.follow_links(true)
|
||||
.follow_links(false)
|
||||
.max_depth(Some(max_depth + 1));
|
||||
|
||||
for entry in builder.build().flatten() {
|
||||
if entry.file_type().is_some_and(|ft| ft.is_symlink()) {
|
||||
continue;
|
||||
}
|
||||
let depth = entry.depth();
|
||||
if depth == 0 || depth > max_depth {
|
||||
continue;
|
||||
@@ -716,6 +722,22 @@ mod project_mapping_tests {
|
||||
assert_eq!(project_tree(root, 1), project_tree(root, 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(unix)]
|
||||
fn project_mapping_does_not_follow_symlinked_key_files() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
let outside = tmp.path().join("outside");
|
||||
fs::create_dir_all(&root).expect("mkdir workspace");
|
||||
fs::create_dir_all(&outside).expect("mkdir outside");
|
||||
let outside_file = outside.join("Cargo.toml");
|
||||
fs::write(&outside_file, "[package]\nname = \"outside\"\n").expect("write outside");
|
||||
std::os::unix::fs::symlink(&outside_file, root.join("Cargo.toml")).expect("symlink");
|
||||
|
||||
assert_eq!(summarize_project(&root), "Unknown project type");
|
||||
assert!(!project_tree(&root, 1).contains("Cargo.toml"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn summarize_project_sorts_key_files_in_fallback() {
|
||||
// When `summarize_project` can't classify a project type it falls
|
||||
|
||||
Reference in New Issue
Block a user