diff --git a/fact/src/host_info.rs b/fact/src/host_info.rs index b00531d4..ffcb9f9e 100644 --- a/fact/src/host_info.rs +++ b/fact/src/host_info.rs @@ -3,10 +3,11 @@ use log::{debug, warn}; use std::{ collections::HashMap, env, - ffi::{CStr, CString, c_char}, + ffi::{CStr, CString, OsStr, c_char}, fs::{File, read_to_string}, io::{BufRead, BufReader}, mem, + os::unix::ffi::OsStrExt, path::{Path, PathBuf}, sync::LazyLock, }; @@ -31,13 +32,14 @@ pub fn prepend_host_mount(path: &Path) -> PathBuf { get_host_mount().join(path) } -pub fn remove_host_mount(path: &Path) -> PathBuf { +pub fn remove_host_mount(path: &Path) -> &Path { let host_mount = get_host_mount(); - if path.starts_with(host_mount) { - let path = path.strip_prefix(host_mount).unwrap(); - Path::new("/").join(path) + if host_mount != "/" && path.starts_with(host_mount) { + let len = host_mount.as_os_str().as_bytes().len(); + let path = &path.as_os_str().as_bytes()[len..]; + Path::new(OsStr::from_bytes(path)) } else { - path.to_path_buf() + path } } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 1ea00a5d..2ef1a66c 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -20,7 +20,7 @@ use std::{ cell::RefCell, - collections::HashMap, + collections::{HashMap, HashSet}, fs::Metadata, io, ops::{Deref, DerefMut}, @@ -48,7 +48,7 @@ use tokio::{ use crate::{ bpf::Bpf, event::Event, - host_info, + host_info::{self, remove_host_mount}, metrics::host_scanner::{HostScannerMetrics, ScanLabels}, }; @@ -105,6 +105,7 @@ pub struct HostScanner { metrics: HostScannerMetrics, paths_globset: GlobSet, + paths_patterns: Vec, } impl HostScanner { @@ -119,9 +120,8 @@ impl HostScanner { let kernel_inode_map = RefCell::new(bpf.take_inode_map()?); let inode_map = RefCell::new(InodeMap::new()); let (tx, output) = mpsc::channel(100); - let paths_globset = HostScanner::build_globset(paths.borrow().as_slice())?; - let host_scanner = HostScanner { + let mut host_scanner = HostScanner { kernel_inode_map, inode_map, paths, @@ -130,45 +130,50 @@ impl HostScanner { tx, introspection, metrics, - paths_globset, + paths_globset: GlobSet::empty(), + paths_patterns: Vec::new(), }; + host_scanner.reload_paths_config()?; + // Run an initial scan to fill in the inode map host_scanner.scan()?; Ok((host_scanner, output)) } - fn build_globset(paths: &[PathBuf]) -> anyhow::Result { + fn reload_paths_config(&mut self) -> anyhow::Result<()> { + let paths = self.paths.borrow(); let mut builder = GlobSetBuilder::new(); + let mut patterns = Vec::with_capacity(paths.len()); + for p in paths.iter() { + patterns.push(host_info::prepend_host_mount(p)); + let Some(glob_str) = p.to_str() else { bail!("failed to convert path {} to string", p.display()); }; - builder.add( - Glob::new(glob_str) - .with_context(|| format!("invalid glob {}", glob_str)) - .unwrap(), - ); + builder.add(Glob::new(glob_str).with_context(|| format!("invalid glob {}", glob_str))?); } - Ok(builder.build()?) + + self.paths_globset = builder.build()?; + self.paths_patterns = patterns; + + Ok(()) } fn scan(&self) -> anyhow::Result<()> { info!("Host scan started"); let start = Instant::now(); self.metrics.scan_inc(ScanLabels::Scans); - let config = self.paths.borrow(); // Cleanup any items that are either: // * Not configured to be monitored anymore. // * Are configured to be monitored but no longer are found in // the file system. self.inode_map.borrow_mut().retain(|inode, path| { - if config.iter().any(|prefix| path.starts_with(prefix)) - && host_info::prepend_host_mount(path).exists() - { + if self.paths_globset.is_match(&path) && host_info::prepend_host_mount(path).exists() { true } else { let _ = self.kernel_inode_map.borrow_mut().remove(inode); @@ -177,9 +182,8 @@ impl HostScanner { } }); - for pattern in self.paths.borrow().iter() { - let path = host_info::prepend_host_mount(pattern); - self.scan_inner(&path)?; + for path in &self.paths_patterns { + self.scan_inner(path)?; } let duration = start.elapsed(); self.metrics.scan_duration.observe(duration.as_secs_f64()); @@ -264,6 +268,37 @@ impl HostScanner { } } + /// Do a partial scan of any pattern that matches the provided path + /// + /// This includes glob expansion matching and any patterns with a + /// base path (the path up to the first glob special character) that + /// matches the supplied path. + fn scan_partial(&self, path: &Path) -> anyhow::Result<()> { + let scan_prefix_patterns = + self.paths_patterns + .iter() + .enumerate() + .filter_map(|(i, pattern)| { + remove_host_mount(pattern) + .to_str()? + .split(['*', '?', '[', '{']) + .next()? + .starts_with(path.to_str()?) + .then_some(i) + }); + let scan_glob_index = self.paths_globset.matches(path); + + // De-duplicate the indexes + let scan_set = scan_prefix_patterns + .chain(scan_glob_index.iter().copied()) + .collect::>(); + + for pattern in scan_set.iter().map(|index| &self.paths_patterns[*index]) { + self.scan_inner(pattern)?; + } + Ok(()) + } + fn update_entry(&self, path: &Path, metadata: &Metadata) -> anyhow::Result<()> { let inode = inode_key_t { inode: metadata.st_ino(), @@ -271,7 +306,7 @@ impl HostScanner { }; let host_path = host_info::remove_host_mount(path); - self.update_entry_with_inode(inode, host_path)?; + self.update_entry_with_inode(inode, host_path.to_path_buf())?; debug!("Added entry for {}: {inode:?}", path.display()); Ok(()) @@ -491,23 +526,17 @@ You can increase this limit with: } /// Handle a mount being modified in a monitored directory. - /// - /// This should really do a partial scan of the directory where the - /// mount is being changed, but we don't have an easy way to do that - /// at the moment, so we trigger a full scan instead. - fn handle_mount_event(&self) { - if let Err(e) = self.scan() { + fn handle_mount_event(&self, event: &Event) { + if let Err(e) = self.scan_partial(event.get_host_path()) { warn!("Host scan failed: {e:?}"); } } /// Handle symlink events by scanning the filesystem - fn handle_symlink_event(&self) -> anyhow::Result<()> { + fn handle_symlink_event(&self, event: &Event) -> anyhow::Result<()> { // Since `glob` follows symlinks unconditionally, we need to do // so as well. - // - // TODO: do a partial scan of the symlink, rather than a full scan - self.scan() + self.scan_partial(event.get_host_path()) } /// Periodically notify the host scanner main task that a scan needs @@ -576,12 +605,6 @@ You can increase this limit with: warn!("Failed to handle creation event: {e}"); } - // Handle mount events and move on. - if event.is_mount_related() { - self.handle_mount_event(); - continue; - } - if let Some(host_path) = self.get_host_path(Some(event.get_inode())) { self.metrics.scan_inc(ScanLabels::InodeHit); event.set_host_path(host_path); @@ -592,6 +615,12 @@ You can increase this limit with: event.set_old_host_path(host_path); } + // Handle mount events and move on. + if event.is_mount_related() { + self.handle_mount_event(&event); + continue; + } + // Remove inode from the map if event.is_deletion() { self.handle_unlink_event(&event); @@ -603,7 +632,7 @@ You can increase this limit with: } if event.is_symlink() && - let Err(e) = self.handle_symlink_event() { + let Err(e) = self.handle_symlink_event(&event) { warn!("Failed to handle symlink event: {e:?}"); } @@ -640,7 +669,7 @@ You can increase this limit with: } _ = scan_trigger.notified() => self.scan()?, _ = self.paths.changed() => { - self.paths_globset = HostScanner::build_globset(self.paths.borrow().as_slice())?; + self.reload_paths_config()?; self.scan()?; } } diff --git a/tests/test_wildcard.py b/tests/test_wildcard.py index c2a2f01c..f0c2713d 100644 --- a/tests/test_wildcard.py +++ b/tests/test_wildcard.py @@ -11,6 +11,15 @@ from server import EventServer +def reload_config( + fact: docker.models.containers.Container, config: dict, config_file: str +): + with open(config_file, 'w') as f: + yaml.dump(config, f) + fact.kill('SIGHUP') + sleep(0.1) + + @pytest.fixture def wildcard_config( fact: docker.models.containers.Container, @@ -23,12 +32,22 @@ def wildcard_config( f'{monitored_dir}/*.conf', f'{monitored_dir}/**/test-*.log', ] - with open(config_file, 'w') as f: - yaml.dump(config, f) - # reload the config - fact.kill('SIGHUP') - sleep(0.1) + reload_config(fact, config, config_file) + return config, config_file + + +@pytest.fixture +def partial_path_match_wildcard( + fact: docker.models.containers.Container, + fact_config: tuple[dict, str], + ignored_dir: str, +): + config, config_file = fact_config + partial_dir = ignored_dir.rsplit('-', 1)[0] + config['paths'] = [f'{partial_dir}*/**/*', f'{partial_dir}*'] + + reload_config(fact, config, config_file) return config, config_file @@ -197,3 +216,75 @@ def test_multiple_patterns( ] server.wait_events(events) + + +def test_partial_dir_pattern( + partial_path_match_wildcard: tuple[dict, str], + ignored_dir: str, + server: EventServer, +): + process = Process.from_proc() + file = os.path.join(ignored_dir, 'file.txt') + with open(file, 'w') as f: + f.write('This is a test') + + server.wait_events( + [ + Event( + process=process, + event_type=EventType.CREATION, + file=file, + host_path=file, + ) + ] + ) + + +def test_partial_scan_follows_symlink( + fact: docker.models.containers.Container, + fact_config: tuple[dict, str], + monitored_dir: str, + ignored_dir: str, + server: EventServer, +): + """ + When paths are wildcard-only (e.g. monitored_dir/**/*.txt), + creating a symlink under monitored_dir should trigger a partial + scan via prefix matching and start tracking the symlink target. + """ + link = os.path.join(monitored_dir, 'link') + config, config_file = fact_config + config['paths'].extend([link, f'{link}/**/*.txt']) + reload_config(fact, config, config_file) + + target = os.path.join(ignored_dir, 'target.txt') + with open(target, 'w') as f: + f.write('symlink target') + os.symlink(os.path.join('..', os.path.basename(ignored_dir)), link) + + process = Process.from_proc() + + server.wait_events( + [ + Event( + process=process, + event_type=EventType.OPEN, + file=link, + host_path=link, + ) + ] + ) + + with open(target, 'w') as f: + f.write('modified target') + + server.wait_events( + [ + Event( + process=process, + event_type=EventType.OPEN, + file=target, + host_path=os.path.join(link, 'target.txt'), + ) + ] + )