From cea2ac4a2b82b26d6b141c7c1b774e53ee07f0ee Mon Sep 17 00:00:00 2001 From: yacosta738 <33158051+yacosta738@users.noreply.github.com> Date: Wed, 6 May 2026 05:27:29 +0000 Subject: [PATCH 1/3] fix(security): prevent path traversal in NestedGlob search root Hardens the NestedGlob sync type in `src/linker.rs` by validating the search root (`source` field) against the project root. This prevents a malicious configuration from triggering a recursive directory walk outside the intended project boundaries. Validation is implemented using `revalidate_destination_path` which correctly handles current directory ('.') search roots while rejecting traversal components or absolute paths that escape the project root. Security regression tests are added in `tests/test_security.rs` to verify the fix. Under 50 lines changed. --- src/linker.rs | 7 +++++++ tests/test_security.rs | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 tests/test_security.rs diff --git a/src/linker.rs b/src/linker.rs index e8855ba5..aa13b39f 100644 --- a/src/linker.rs +++ b/src/linker.rs @@ -388,6 +388,9 @@ impl Linker { // For NestedGlob, `source` is relative to the project root (not source_dir). let search_root = self.project_root.join(&target.source); + // SECURITY: Validate search root to prevent traversal/absolute escapes. + self.revalidate_destination_path(&search_root)?; + self.process_nested_glob( &search_root, target.pattern.as_deref().unwrap_or("**/AGENTS.md"), @@ -1150,6 +1153,10 @@ impl Linker { // Re-discover the same files and remove the corresponding symlinks. let search_root = self.project_root.join(&target_config.source); + // SECURITY: Validate search root to prevent traversal/absolute escapes. + if self.revalidate_destination_path(&search_root).is_err() { + continue; + } if !search_root.exists() || !search_root.is_dir() { continue; } diff --git a/tests/test_security.rs b/tests/test_security.rs new file mode 100644 index 00000000..35514f11 --- /dev/null +++ b/tests/test_security.rs @@ -0,0 +1,47 @@ + +use agentsync::config::Config; +use agentsync::linker::{Linker, SyncOptions}; +use std::fs; +use tempfile::TempDir; + +#[test] +fn test_nested_glob_search_root_traversal() { + let temp_dir = TempDir::new().unwrap(); + let project_root = temp_dir.path().join("project"); + let agents_dir = project_root.join(".agents"); + fs::create_dir_all(&agents_dir).unwrap(); + + // Create a file OUTSIDE the project root + let outside_dir = temp_dir.path().join("outside_dir"); + fs::create_dir_all(&outside_dir).unwrap(); + fs::write(outside_dir.join("AGENTS.md"), "outside").unwrap(); + + let config_path = agents_dir.join("agentsync.toml"); + + // We want to walk a directory OUTSIDE the project root + let relative_outside = "../outside_dir"; + + let toml = format!(r#" + source_dir = "." + [agents.malicious] + enabled = true + [agents.malicious.targets.nested] + source = "{}" + destination = "leaked/{{file_name}}" + type = "nested-glob" + "#, relative_outside); + + fs::write(&config_path, toml).unwrap(); + + let config = Config::load(&config_path).unwrap(); + let linker = Linker::new(config, config_path); + + let options = SyncOptions { verbose: true, ..Default::default() }; + let result = linker.sync(&options).unwrap(); + + // The target should have failed due to unsafe search root + assert!(result.errors > 0, "Sync should have errors for malicious search root"); + + let leaked_link = project_root.join("leaked").join("AGENTS.md"); + assert!(!leaked_link.exists(), "Should NOT have created a symlink to a file discovered outside project root"); +} From 597f8699ff779cd1ba73ebccc790edbef43e8503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 8 May 2026 14:36:40 +0200 Subject: [PATCH 2/3] fix: address security PR review feedback --- src/linker.rs | 8 +++++- tests/test_security.rs | 59 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/linker.rs b/src/linker.rs index aa13b39f..dc0b0286 100644 --- a/src/linker.rs +++ b/src/linker.rs @@ -389,7 +389,13 @@ impl Linker { // For NestedGlob, `source` is relative to the project root (not source_dir). let search_root = self.project_root.join(&target.source); // SECURITY: Validate search root to prevent traversal/absolute escapes. - self.revalidate_destination_path(&search_root)?; + self.revalidate_destination_path(&search_root) + .with_context(|| { + format!( + "NestedGlob source resolves outside project root: {}", + target.source + ) + })?; self.process_nested_glob( &search_root, diff --git a/tests/test_security.rs b/tests/test_security.rs index 35514f11..4dbf79d3 100644 --- a/tests/test_security.rs +++ b/tests/test_security.rs @@ -1,4 +1,3 @@ - use agentsync::config::Config; use agentsync::linker::{Linker, SyncOptions}; use std::fs; @@ -21,7 +20,8 @@ fn test_nested_glob_search_root_traversal() { // We want to walk a directory OUTSIDE the project root let relative_outside = "../outside_dir"; - let toml = format!(r#" + let toml = format!( + r#" source_dir = "." [agents.malicious] enabled = true @@ -29,19 +29,64 @@ fn test_nested_glob_search_root_traversal() { source = "{}" destination = "leaked/{{file_name}}" type = "nested-glob" - "#, relative_outside); + "#, + relative_outside + ); fs::write(&config_path, toml).unwrap(); let config = Config::load(&config_path).unwrap(); - let linker = Linker::new(config, config_path); + let linker = Linker::new(config, config_path.clone()); - let options = SyncOptions { verbose: true, ..Default::default() }; + let options = SyncOptions { + verbose: true, + ..Default::default() + }; let result = linker.sync(&options).unwrap(); // The target should have failed due to unsafe search root - assert!(result.errors > 0, "Sync should have errors for malicious search root"); + assert!( + result.errors > 0, + "Sync should have errors for malicious search root" + ); let leaked_link = project_root.join("leaked").join("AGENTS.md"); - assert!(!leaked_link.exists(), "Should NOT have created a symlink to a file discovered outside project root"); + assert!( + !leaked_link.exists(), + "Should NOT have created a symlink to a file discovered outside project root" + ); + assert!( + !project_root.join("leaked").exists(), + "Should NOT have created the leaked directory" + ); + + // Absolute paths should also be rejected. + let absolute_toml = format!( + r#" + source_dir = "." + [agents.malicious] + enabled = true + [agents.malicious.targets.nested] + source = "{}" + destination = "leaked/{{file_name}}" + type = "nested-glob" + "#, + outside_dir.display() + ); + fs::write(&config_path, absolute_toml).unwrap(); + + let absolute_config = Config::load(&config_path).unwrap(); + let absolute_linker = Linker::new(absolute_config, config_path); + let absolute_result = absolute_linker.sync(&options).unwrap(); + assert!( + absolute_result.errors > 0, + "Sync should have errors for absolute-path search root" + ); + + // clean() must not traverse or remove anything outside the project. + let clean_result = absolute_linker.clean(&SyncOptions::default()).unwrap(); + assert_eq!( + clean_result.removed, 0, + "Clean should not remove anything for an invalid search root" + ); } From b20a90f69501a437f96ee330c129ae31094c780a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 11 May 2026 14:39:21 +0000 Subject: [PATCH 3/3] fix(test): escape backslashes in absolute path TOML string for Windows compatibility Agent-Logs-Url: https://github.com/dallay/agentsync/sessions/a8b3f8b6-f5cd-4ac2-9373-a959808eb383 Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com> --- tests/test_security.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_security.rs b/tests/test_security.rs index 4dbf79d3..27611341 100644 --- a/tests/test_security.rs +++ b/tests/test_security.rs @@ -61,6 +61,8 @@ fn test_nested_glob_search_root_traversal() { ); // Absolute paths should also be rejected. + // Use replace to escape backslashes so the path is valid in a TOML basic string on Windows. + let path_str = outside_dir.display().to_string().replace('\\', "\\\\"); let absolute_toml = format!( r#" source_dir = "." @@ -71,7 +73,7 @@ fn test_nested_glob_search_root_traversal() { destination = "leaked/{{file_name}}" type = "nested-glob" "#, - outside_dir.display() + path_str ); fs::write(&config_path, absolute_toml).unwrap();