fix(guards): preserve relative glob and exception compatibility

Author: bb-connor

crates/libs/clawdstrike/src/guards/forbidden_path.rs

@@ -4,7 +4,10 @@ use async_trait::async_trait;
 use glob::Pattern;
 use serde::{Deserialize, Serialize};
 
-use super::path_normalization::{normalize_path_for_policy, normalize_path_for_policy_with_fs};
+use super::path_normalization::{
+    normalize_path_for_policy, normalize_path_for_policy_lexical_absolute,
+    normalize_path_for_policy_with_fs,
+};
 use super::{Guard, GuardAction, GuardContext, GuardResult, Severity};
 
 /// Configuration for ForbiddenPathGuard
@@ -191,16 +194,28 @@ impl ForbiddenPathGuard {
     pub fn is_forbidden(&self, path: &str) -> bool {
         let lexical_path = normalize_path_for_policy(path);
         let resolved_path = normalize_path_for_policy_with_fs(path);
-        let resolved_differs = resolved_path != lexical_path;
+        let lexical_abs_path = normalize_path_for_policy_lexical_absolute(path);
+        let resolved_differs_from_lexical_target = lexical_abs_path
+            .as_deref()
+            .map(|abs| abs != resolved_path.as_str())
+            .unwrap_or(resolved_path != lexical_path);
 
         // Check exceptions first
         for exception in &self.exceptions {
-            let exception_matches = if resolved_differs {
-                // If resolution changed the path (e.g., via symlink/canonical host mount aliases),
-                // require the exception to match the resolved target to avoid lexical bypasses.
-                exception.matches(&resolved_path)
+            let lexical_matches = exception.matches(&lexical_path)
+                || lexical_abs_path
+                    .as_deref()
+                    .map(|abs| exception.matches(abs))
+                    .unwrap_or(false);
+            let resolved_matches = exception.matches(&resolved_path);
+            let exception_matches = if resolved_differs_from_lexical_target {
+                // If resolution changed the actual target (for example via symlink traversal),
+                // require the exception to match the resolved target to prevent lexical bypasses.
+                resolved_matches
             } else {
-                exception.matches(&lexical_path)
+                // If target identity is unchanged (for example relative -> absolute conversion),
+                // allow either lexical or resolved exception forms.
+                resolved_matches || lexical_matches
             };
 
             if exception_matches {
@@ -309,6 +324,29 @@ mod tests {
         assert!(!guard.is_forbidden("/app/project/.env"));
     }
 
+    #[test]
+    fn relative_exception_matches_when_target_is_unchanged() {
+        let rel_dir = format!("target/forbidden-path-rel-{}", uuid::Uuid::new_v4());
+        std::fs::create_dir_all(&rel_dir).expect("create rel dir");
+        let rel_file = format!("{rel_dir}/.env");
+        std::fs::write(&rel_file, "API_KEY=test\n").expect("write file");
+
+        let guard = ForbiddenPathGuard::with_config(ForbiddenPathConfig {
+            enabled: true,
+            patterns: Some(vec!["**/.env".to_string()]),
+            exceptions: vec![rel_file.clone()],
+            additional_patterns: vec![],
+            remove_patterns: vec![],
+        });
+
+        assert!(
+            !guard.is_forbidden(&rel_file),
+            "relative exception should match even when fs normalization produces absolute path"
+        );
+
+        let _ = std::fs::remove_dir_all(&rel_dir);
+    }
+
     #[test]
     fn test_additional_patterns_field() {
         let yaml = r#"
@@ -398,4 +436,36 @@ remove_patterns:
 
         let _ = std::fs::remove_dir_all(&root);
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn lexical_exception_does_not_bypass_forbidden_resolved_target() {
+        use std::os::unix::fs::symlink;
+
+        let root = std::env::temp_dir().join(format!("forbidden-path-{}", uuid::Uuid::new_v4()));
+        let safe_dir = root.join("safe");
+        let forbidden_dir = root.join("forbidden");
+        std::fs::create_dir_all(&safe_dir).expect("create safe dir");
+        std::fs::create_dir_all(&forbidden_dir).expect("create forbidden dir");
+
+        let target = forbidden_dir.join("secret.env");
+        std::fs::write(&target, "secret").expect("write target");
+        let link = safe_dir.join("project.env");
+        symlink(&target, &link).expect("create symlink");
+
+        let guard = ForbiddenPathGuard::with_config(ForbiddenPathConfig {
+            enabled: true,
+            patterns: Some(vec!["**/forbidden/**".to_string()]),
+            exceptions: vec!["**/safe/project.env".to_string()],
+            additional_patterns: vec![],
+            remove_patterns: vec![],
+        });
+
+        assert!(
+            guard.is_forbidden(link.to_str().expect("utf-8 path")),
+            "lexical-only exception should not bypass when resolved target is forbidden"
+        );
+
+        let _ = std::fs::remove_dir_all(&root);
+    }
 }

crates/libs/clawdstrike/src/guards/path_allowlist.rs

@@ -4,7 +4,10 @@ use async_trait::async_trait;
 use glob::Pattern;
 use serde::{Deserialize, Serialize};
 
-use super::path_normalization::normalize_path_for_policy_with_fs;
+use super::path_normalization::{
+    normalize_path_for_policy, normalize_path_for_policy_lexical_absolute,
+    normalize_path_for_policy_with_fs,
+};
 use super::{Guard, GuardAction, GuardContext, GuardResult, Severity};
 
 /// Configuration for `PathAllowlistGuard`.
@@ -95,28 +98,49 @@ impl PathAllowlistGuard {
         patterns.iter().any(|p| p.matches(path))
     }
 
+    fn matches_allowlist(&self, patterns: &[Pattern], path: &str) -> bool {
+        let lexical_path = normalize_path_for_policy(path);
+        let resolved_path = normalize_path_for_policy_with_fs(path);
+        let lexical_abs_path = normalize_path_for_policy_lexical_absolute(path);
+
+        let resolved_differs_from_lexical_target = lexical_abs_path
+            .as_deref()
+            .map(|abs| abs != resolved_path.as_str())
+            .unwrap_or(resolved_path != lexical_path);
+
+        if resolved_differs_from_lexical_target {
+            // When resolution changes the target (for example, symlink traversal), require the
+            // resolved path to match to prevent lexical-path allowlist bypasses.
+            return Self::matches_any(patterns, &resolved_path);
+        }
+
+        Self::matches_any(patterns, &lexical_path)
+            || Self::matches_any(patterns, &resolved_path)
+            || lexical_abs_path
+                .as_deref()
+                .map(|abs| Self::matches_any(patterns, abs))
+                .unwrap_or(false)
+    }
+
     pub fn is_file_access_allowed(&self, path: &str) -> bool {
         if !self.enabled {
             return true;
         }
-        let normalized = normalize_path_for_policy_with_fs(path);
-        Self::matches_any(&self.file_access_allow, &normalized)
+        self.matches_allowlist(&self.file_access_allow, path)
     }
 
     pub fn is_file_write_allowed(&self, path: &str) -> bool {
         if !self.enabled {
             return true;
         }
-        let normalized = normalize_path_for_policy_with_fs(path);
-        Self::matches_any(&self.file_write_allow, &normalized)
+        self.matches_allowlist(&self.file_write_allow, path)
     }
 
     pub fn is_patch_allowed(&self, path: &str) -> bool {
         if !self.enabled {
             return true;
         }
-        let normalized = normalize_path_for_policy_with_fs(path);
-        Self::matches_any(&self.patch_allow, &normalized)
+        self.matches_allowlist(&self.patch_allow, path)
     }
 }
 
@@ -222,6 +246,36 @@ mod tests {
         assert!(!guard.is_patch_allowed("/tmp/other/src/main.rs"));
     }
 
+    #[test]
+    fn relative_allowlist_globs_match_when_target_is_unchanged() {
+        let rel_dir = format!("target/path-allowlist-rel-{}", uuid::Uuid::new_v4());
+        std::fs::create_dir_all(&rel_dir).expect("create rel dir");
+        let rel_file = format!("{rel_dir}/main.rs");
+        std::fs::write(&rel_file, "fn main() {}\n").expect("write file");
+
+        let guard = PathAllowlistGuard::with_config(PathAllowlistConfig {
+            enabled: true,
+            file_access_allow: vec![format!("{rel_dir}/**")],
+            file_write_allow: vec![format!("{rel_dir}/**")],
+            patch_allow: vec![format!("{rel_dir}/**")],
+        });
+
+        assert!(
+            guard.is_file_access_allowed(&rel_file),
+            "relative file-access allowlist glob should match"
+        );
+        assert!(
+            guard.is_file_write_allowed(&rel_file),
+            "relative file-write allowlist glob should match"
+        );
+        assert!(
+            guard.is_patch_allowed(&rel_file),
+            "relative patch allowlist glob should match"
+        );
+
+        let _ = std::fs::remove_dir_all(&rel_dir);
+    }
+
     #[cfg(unix)]
     #[test]
     fn symlink_escape_outside_allowlist_is_denied() {

crates/libs/clawdstrike/src/guards/path_normalization.rs

@@ -48,6 +48,21 @@ pub fn normalize_path_for_policy(path: &str) -> String {
     }
 }
 
+/// Normalize a path to an absolute lexical path without resolving symlinks.
+///
+/// - Absolute inputs are normalized as-is.
+/// - Relative inputs are joined against the current working directory.
+pub fn normalize_path_for_policy_lexical_absolute(path: &str) -> Option<String> {
+    let raw = Path::new(path);
+    if raw.is_absolute() {
+        return Some(normalize_path_for_policy(path));
+    }
+
+    let cwd = std::env::current_dir().ok()?;
+    let joined = cwd.join(raw);
+    Some(normalize_path_for_policy(&joined.to_string_lossy()))
+}
+
 /// Normalize a path for policy matching, preferring filesystem-resolved targets when possible.
 ///
 /// - For existing paths, this resolves symlinks via `canonicalize`.
@@ -74,7 +89,10 @@ fn resolve_path_for_policy(path: &str) -> Option<String> {
 
 #[cfg(test)]
 mod tests {
-    use super::{normalize_path_for_policy, normalize_path_for_policy_with_fs};
+    use super::{
+        normalize_path_for_policy, normalize_path_for_policy_lexical_absolute,
+        normalize_path_for_policy_with_fs,
+    };
 
     #[test]
     fn normalizes_separators_and_dots() {
@@ -108,4 +126,19 @@ mod tests {
         );
         let _ = std::fs::remove_dir_all(&root);
     }
+
+    #[test]
+    fn lexical_absolute_normalization_keeps_relative_shape_without_resolving_links() {
+        let relative = format!(
+            "target/path-normalization-{}/file.txt",
+            uuid::Uuid::new_v4()
+        );
+        let expected = {
+            let cwd = std::env::current_dir().expect("cwd");
+            normalize_path_for_policy(&cwd.join(&relative).to_string_lossy())
+        };
+        let actual = normalize_path_for_policy_lexical_absolute(&relative)
+            .expect("lexical absolute normalization should succeed");
+        assert_eq!(actual, expected);
+    }
 }