test: add comprehensive test coverage for filter module

- Add tests for FilterPolicy.from_config() with glob patterns and prefixes
- Add tests for FilterPolicy accessor methods (rule_count, default_action)
- Add tests for FilterRule.matches() with all conditions
- Add tests for SharedFilterPolicy (check_with_dest, From impl)
- Add tests for Operation.all() and additional parse/display tests
- Add tests for matcher accessor methods (prefix, path, component, extension)
- Add tests for GlobMatchMode enum and CombinedMatcher len/is_empty
- Fix normalize_path tests placement (move inside test module)
- Update ARCHITECTURE.md with File Transfer Filter module documentation

Author: inureyes

ARCHITECTURE.md

@@ -213,6 +213,107 @@ Security features for the SSH server (`src/server/security/`):
   - Thread-safe with fail-closed behavior on lock contention
   - Configuration via `allowed_ips` and `blocked_ips` in server config
 
+### File Transfer Filter Module
+
+Policy-based filtering infrastructure for SFTP and SCP file transfer operations (`src/server/filter/`):
+
+**Structure**:
+- `mod.rs` - `TransferFilter` trait, `Operation` enum, `FilterResult` enum, `NoOpFilter`
+- `policy.rs` - `FilterPolicy` engine, `FilterRule`, `Matcher` trait, `SharedFilterPolicy`
+- `path.rs` - Path-based matchers: `PrefixMatcher`, `ExactMatcher`, `ComponentMatcher`, `ExtensionMatcher`
+- `pattern.rs` - Pattern-based matchers: `GlobMatcher`, `RegexMatcher`, `CombinedMatcher`, `NotMatcher`
+
+**Key Components**:
+
+- **Operation**: Enum representing file operations
+  - `Upload`, `Download`, `Delete`, `Rename`
+  - `CreateDir`, `ListDir`, `Stat`, `SetStat`
+  - `Symlink`, `ReadLink`
+
+- **FilterResult**: Actions to take on matched operations
+  - `Allow` - Permit the operation (default)
+  - `Deny` - Block the operation
+  - `Log` - Allow but log for auditing
+
+- **TransferFilter Trait**: Interface for custom filter implementations
+  - `check(path, operation, user)` - Check single path operations
+  - `check_with_dest(src, dest, operation, user)` - Check two-path operations (rename, symlink)
+  - `is_enabled()` - Check if filtering is active
+
+- **FilterPolicy**: First-match-wins rule evaluation engine
+  - Ordered rule evaluation
+  - Configurable default action
+  - Enable/disable filtering
+  - Create from YAML configuration via `from_config()`
+
+- **FilterRule**: Combines matcher, action, and optional constraints
+  - Path pattern matcher
+  - Per-operation restrictions
+  - Per-user restrictions
+  - Named rules for debugging
+
+**Built-in Matchers**:
+
+| Matcher | Purpose | Example |
+|---------|---------|---------|
+| `GlobMatcher` | Wildcard patterns | `*.key`, `*.pem` |
+| `RegexMatcher` | Full regex support | `(?i)\.exe$` |
+| `PrefixMatcher` | Directory tree matching | `/etc/` |
+| `ExactMatcher` | Specific file matching | `/etc/shadow` |
+| `ComponentMatcher` | Path component matching | `.git`, `.ssh` |
+| `ExtensionMatcher` | File extension matching | `exe`, `key` |
+| `CombinedMatcher` | OR-combine matchers | Multiple patterns |
+| `NotMatcher` | Invert matcher results | Exclude patterns |
+
+**Security Features**:
+- `normalize_path()` function for path traversal prevention
+- ReDoS protection via regex size limits
+- Case-insensitive extension matching
+
+**Usage Example**:
+```rust
+use bssh::server::filter::{FilterPolicy, FilterResult, Operation};
+use bssh::server::filter::pattern::GlobMatcher;
+use bssh::server::filter::policy::FilterRule;
+use std::path::Path;
+
+// Create policy that blocks *.key files
+let policy = FilterPolicy::new()
+    .with_default(FilterResult::Allow)
+    .add_rule(FilterRule::new(
+        Box::new(GlobMatcher::new("*.key").unwrap()),
+        FilterResult::Deny,
+    ));
+
+// Check if operation is allowed
+let result = policy.check(
+    Path::new("/etc/secret.key"),
+    Operation::Download,
+    "alice"
+);
+assert_eq!(result, FilterResult::Deny);
+```
+
+**Configuration** (YAML):
+```yaml
+filter:
+  enabled: true
+  default_action: allow
+  rules:
+    - name: block-sensitive-keys
+      pattern: "*.{key,pem}"
+      action: deny
+      operations:
+        - download
+        - upload
+    - name: block-hidden-dirs
+      path_prefix: "/home"
+      pattern: ".*"
+      action: deny
+      users:
+        - guest
+```
+
 ### Audit Logging Module
 
 Comprehensive audit logging infrastructure for the SSH server (`src/server/audit/`):

src/server/filter/mod.rs

@@ -161,7 +161,6 @@ pub enum FilterResult {
     Log,
 }
 
-
 impl fmt::Display for FilterResult {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -262,7 +261,10 @@ mod tests {
     #[test]
     fn test_operation_parse() {
         assert_eq!("upload".parse::<Operation>().unwrap(), Operation::Upload);
-        assert_eq!("DOWNLOAD".parse::<Operation>().unwrap(), Operation::Download);
+        assert_eq!(
+            "DOWNLOAD".parse::<Operation>().unwrap(),
+            Operation::Download
+        );
         assert_eq!("mkdir".parse::<Operation>().unwrap(), Operation::CreateDir);
         assert_eq!("readdir".parse::<Operation>().unwrap(), Operation::ListDir);
         assert!("invalid".parse::<Operation>().is_err());
@@ -376,4 +378,62 @@ mod tests {
             FilterResult::Log
         );
     }
+
+    #[test]
+    fn test_operation_all() {
+        let all_ops = Operation::all();
+
+        // Should contain all 10 operations
+        assert_eq!(all_ops.len(), 10);
+
+        // Verify all operations are included
+        assert!(all_ops.contains(&Operation::Upload));
+        assert!(all_ops.contains(&Operation::Download));
+        assert!(all_ops.contains(&Operation::Delete));
+        assert!(all_ops.contains(&Operation::Rename));
+        assert!(all_ops.contains(&Operation::CreateDir));
+        assert!(all_ops.contains(&Operation::ListDir));
+        assert!(all_ops.contains(&Operation::Stat));
+        assert!(all_ops.contains(&Operation::SetStat));
+        assert!(all_ops.contains(&Operation::Symlink));
+        assert!(all_ops.contains(&Operation::ReadLink));
+    }
+
+    #[test]
+    fn test_operation_display_all() {
+        // Test all operations have a string representation
+        assert_eq!(Operation::Stat.to_string(), "stat");
+        assert_eq!(Operation::SetStat.to_string(), "setstat");
+        assert_eq!(Operation::Symlink.to_string(), "symlink");
+        assert_eq!(Operation::ReadLink.to_string(), "readlink");
+    }
+
+    #[test]
+    fn test_operation_parse_all_variants() {
+        // Test parsing all valid variants
+        assert_eq!("stat".parse::<Operation>().unwrap(), Operation::Stat);
+        assert_eq!("setstat".parse::<Operation>().unwrap(), Operation::SetStat);
+        assert_eq!("symlink".parse::<Operation>().unwrap(), Operation::Symlink);
+        assert_eq!(
+            "readlink".parse::<Operation>().unwrap(),
+            Operation::ReadLink
+        );
+
+        // Test case insensitivity
+        assert_eq!("STAT".parse::<Operation>().unwrap(), Operation::Stat);
+        assert_eq!("SetStat".parse::<Operation>().unwrap(), Operation::SetStat);
+    }
+
+    #[test]
+    fn test_noop_filter_default() {
+        let filter = NoOpFilter::default();
+        assert!(!filter.is_enabled());
+    }
+
+    #[test]
+    fn test_noop_filter_clone() {
+        let filter = NoOpFilter;
+        let cloned = filter.clone();
+        assert!(!cloned.is_enabled());
+    }
 }

src/server/filter/path.rs

@@ -81,7 +81,7 @@ use super::policy::Matcher;
 /// ```
 pub fn normalize_path(path: &Path) -> PathBuf {
     let mut result = PathBuf::new();
-    
+
     for component in path.components() {
         match component {
             Component::Prefix(p) => result.push(p.as_os_str()),
@@ -99,7 +99,7 @@ pub fn normalize_path(path: &Path) -> PathBuf {
             Component::Normal(name) => result.push(name),
         }
     }
-    
+
     if result.as_os_str().is_empty() {
         PathBuf::from(".")
     } else {
@@ -495,25 +495,39 @@ mod tests {
         assert!(component.matches(test_path));
         assert!(extension.matches(test_path));
     }
-}
 
     #[test]
     fn test_normalize_path_removes_dot() {
-        assert_eq!(normalize_path(Path::new("/etc/./passwd")), Path::new("/etc/passwd"));
-        assert_eq!(normalize_path(Path::new("./foo/./bar")), Path::new("foo/bar"));
+        assert_eq!(
+            normalize_path(Path::new("/etc/./passwd")),
+            Path::new("/etc/passwd")
+        );
+        assert_eq!(
+            normalize_path(Path::new("./foo/./bar")),
+            Path::new("foo/bar")
+        );
     }
 
     #[test]
     fn test_normalize_path_resolves_parent() {
         assert_eq!(normalize_path(Path::new("/etc/../var")), Path::new("/var"));
-        assert_eq!(normalize_path(Path::new("/etc/ssh/../passwd")), Path::new("/etc/passwd"));
-        assert_eq!(normalize_path(Path::new("/a/b/c/../../d")), Path::new("/a/d"));
+        assert_eq!(
+            normalize_path(Path::new("/etc/ssh/../passwd")),
+            Path::new("/etc/passwd")
+        );
+        assert_eq!(
+            normalize_path(Path::new("/a/b/c/../../d")),
+            Path::new("/a/d")
+        );
     }
 
     #[test]
     fn test_normalize_path_traversal_at_root() {
         // At root, .. should be ignored
-        assert_eq!(normalize_path(Path::new("/../etc/passwd")), Path::new("/etc/passwd"));
+        assert_eq!(
+            normalize_path(Path::new("/../etc/passwd")),
+            Path::new("/etc/passwd")
+        );
         assert_eq!(normalize_path(Path::new("/../../etc")), Path::new("/etc"));
     }
 
@@ -533,13 +547,39 @@ mod tests {
     fn test_normalize_path_security() {
         // This is the key security test: path traversal should be normalized
         let matcher = PrefixMatcher::new("/etc");
-        
+
         // Without normalization, this would NOT match /etc (attack succeeds)
         let attack_path = Path::new("/var/../etc/passwd");
         assert!(!matcher.matches(attack_path)); // Raw path doesn't match
-        
+
         // With normalization, it correctly matches /etc (attack blocked)
         let normalized = normalize_path(attack_path);
         assert!(matcher.matches(&normalized)); // Normalized path matches
         assert_eq!(normalized, Path::new("/etc/passwd"));
     }
+
+    #[test]
+    fn test_prefix_matcher_accessor() {
+        let matcher = PrefixMatcher::new("/home/user");
+        assert_eq!(matcher.prefix(), Path::new("/home/user"));
+    }
+
+    #[test]
+    fn test_exact_matcher_accessor() {
+        let matcher = ExactMatcher::new("/etc/shadow");
+        assert_eq!(matcher.path(), Path::new("/etc/shadow"));
+    }
+
+    #[test]
+    fn test_component_matcher_accessor() {
+        let matcher = ComponentMatcher::new(".git");
+        assert_eq!(matcher.component(), ".git");
+    }
+
+    #[test]
+    fn test_extension_matcher_accessor() {
+        let matcher = ExtensionMatcher::new("PDF");
+        // Extension is stored in lowercase
+        assert_eq!(matcher.extension(), "pdf");
+    }
+}

src/server/filter/pattern.rs

@@ -176,12 +176,8 @@ impl Matcher for GlobMatcher {
                 // Also try matching just the filename for patterns like "*.key"
                 self.matches_filename(path)
             }
-            GlobMatchMode::FullPathOnly => {
-                self.pattern.matches_path(path)
-            }
-            GlobMatchMode::FilenameOnly => {
-                self.matches_filename(path)
-            }
+            GlobMatchMode::FullPathOnly => self.pattern.matches_path(path),
+            GlobMatchMode::FilenameOnly => self.matches_filename(path),
         }
     }
 
@@ -363,7 +359,11 @@ impl Matcher for CombinedMatcher {
     }
 
     fn pattern_description(&self) -> String {
-        let descriptions: Vec<_> = self.matchers.iter().map(|m| m.pattern_description()).collect();
+        let descriptions: Vec<_> = self
+            .matchers
+            .iter()
+            .map(|m| m.pattern_description())
+            .collect();
         format!("any_of:[{}]", descriptions.join(", "))
     }
 }
@@ -630,7 +630,7 @@ mod tests {
         let matcher = GlobMatcher::with_mode("*.key", GlobMatchMode::FullPathOnly).unwrap();
 
         assert!(matcher.matches(Path::new("secret.key"))); // Direct match
-        // * in glob matches path separators too, so this actually matches
+                                                           // * in glob matches path separators too, so this actually matches
         assert!(matcher.matches(Path::new("/etc/secret.key")));
     }
 
@@ -653,4 +653,41 @@ mod tests {
         assert!(matcher.matches(Path::new("/etc/ssl/private.key"))); // Matches via filename
         assert_eq!(matcher.mode(), GlobMatchMode::PathOrFilename);
     }
+
+    #[test]
+    fn test_glob_matcher_pattern_accessor() {
+        let matcher = GlobMatcher::new("*.{key,pem}").unwrap();
+        assert_eq!(matcher.pattern(), "*.{key,pem}");
+    }
+
+    #[test]
+    fn test_regex_matcher_pattern_accessor() {
+        let matcher = RegexMatcher::new(r"(?i)\.key$").unwrap();
+        assert_eq!(matcher.pattern(), r"(?i)\.key$");
+    }
+
+    #[test]
+    fn test_combined_matcher_len_and_is_empty() {
+        let empty = CombinedMatcher::new(vec![]);
+        assert!(empty.is_empty());
+        assert_eq!(empty.len(), 0);
+
+        let non_empty = CombinedMatcher::new(vec![
+            Box::new(GlobMatcher::new("*.key").unwrap()),
+            Box::new(GlobMatcher::new("*.pem").unwrap()),
+        ]);
+        assert!(!non_empty.is_empty());
+        assert_eq!(non_empty.len(), 2);
+    }
+
+    #[test]
+    fn test_glob_match_mode_enum() {
+        // Test that GlobMatchMode implements Default correctly
+        assert_eq!(GlobMatchMode::default(), GlobMatchMode::PathOrFilename);
+
+        // Test each mode
+        assert_ne!(GlobMatchMode::PathOrFilename, GlobMatchMode::FullPathOnly);
+        assert_ne!(GlobMatchMode::PathOrFilename, GlobMatchMode::FilenameOnly);
+        assert_ne!(GlobMatchMode::FullPathOnly, GlobMatchMode::FilenameOnly);
+    }
 }