From 1187dead49c3b9d6394f21a1ba0beaa1c5de8d21 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:21:26 -0800 Subject: [PATCH 1/5] fix(index): admit explicitly included Git submodules --- .../tracedecay-code-index/src/source_walk.rs | 238 +++++++++++++++++- 1 file changed, 230 insertions(+), 8 deletions(-) diff --git a/crates/tracedecay-code-index/src/source_walk.rs b/crates/tracedecay-code-index/src/source_walk.rs index add7683673..944fbf18e0 100644 --- a/crates/tracedecay-code-index/src/source_walk.rs +++ b/crates/tracedecay-code-index/src/source_walk.rs @@ -9,7 +9,8 @@ //! walk is therefore public rather than crate-private, the audit in the root //! crate reuses this policy instead of restating it. -use std::path::{Path, PathBuf}; +use std::collections::BTreeSet; +use std::path::{Component, Path, PathBuf}; use std::sync::Arc; use ignore::overrides::{Override, OverrideBuilder}; @@ -116,6 +117,7 @@ pub fn source_walk( let generated_dir_overrides = overrides.clone(); let path_glob_scope = path_glob.and_then(PathGlobScope::from_path_glob); let filter_root = project_root.to_path_buf(); + let included_submodules = included_git_submodules(project_root, path_policy)?; let path_policy = path_policy.clone(); let mut builder = WalkBuilder::new(project_root); @@ -165,13 +167,13 @@ pub fn source_walk( if !is_dir { return !path_policy.excludes(&relative); } - // A directory with its own Git authority is another project, not - // source owned by this one. This covers linked worktrees (`.git` - // file), nested clones/submodules (`.git` directory), and keeps a - // primary checkout containing agent worktrees from indexing many - // copies of itself. The project root is deliberately exempt at - // depth zero above. - if std::fs::symlink_metadata(entry.path().join(".git")).is_ok() { + // Nested Git repositories are independent sources except for + // authenticated submodules selected by an explicit index include. + // Linked worktrees and unrelated clones remain pruned even when + // a broad source glob would otherwise include their contents. + if std::fs::symlink_metadata(entry.path().join(".git")).is_ok() + && !included_submodules.contains(Path::new(&relative)) + { return false; } !path_policy.excludes_directory(&relative) @@ -193,6 +195,160 @@ pub fn forward_slash_relative(relative: &Path) -> Arc { tracedecay_domain::forward_slash_path(relative).into() } +/// A nested repository is part of this source tree only when an explicit +/// index.include.v1 path selects a registered, checked-out Git submodule. +/// Linked worktrees and unrelated clones also carry .git files, so the +/// marker alone never authorizes traversing their contents. +fn included_git_submodules( + project_root: &Path, + policy: &IndexPathPolicyV1, +) -> Result, SourceWalkError> { + if policy.include_patterns().is_empty() { + return Ok(BTreeSet::new()); + } + let modules_file = project_root.join(".gitmodules"); + match std::fs::symlink_metadata(&modules_file) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(BTreeSet::new()); + } + Ok(metadata) if metadata.file_type().is_file() => {} + Ok(_) => return Err(git_walk_error(".gitmodules is not a regular file")), + Err(error) => return Err(git_walk_error(error)), + } + + let mut command = source_git_command(project_root)?; + command.args([ + "config", + "--null", + "--file", + ".gitmodules", + "--get-regexp", + r"^submodule\..*\.path$", + ]); + let modules = command.output().map_err(git_walk_error)?; + // Git config returns exit 1 when there are no matching keys. + if modules.status.code() == Some(1) { + return Ok(BTreeSet::new()); + } + if !modules.status.success() { + return Err(git_walk_error(String::from_utf8_lossy(&modules.stderr))); + } + let registered = modules + .stdout + .split(|byte| *byte == 0) + .filter_map(|record| { + let separator = record.iter().position(|byte| *byte == b'\n')?; + let key = std::str::from_utf8(&record[..separator]).ok()?; + if !key.starts_with("submodule.") || !key.ends_with(".path") { + return None; + } + let path = std::str::from_utf8(&record[separator + 1..]).ok()?; + let relative = Path::new(path); + if path.is_empty() + || relative.is_absolute() + || !relative.components().all(|part| matches!(part, Component::Normal(_))) + { + return None; + } + let explicitly_included = policy.include_patterns().iter().any(|rule| { + let rule = rule.trim_start_matches("./").trim_start_matches('/'); + let rule = rule.trim_end_matches('/'); + rule == path || rule.strip_suffix("/**") == Some(path) + }); + explicitly_included.then(|| path.to_owned()) + }) + .collect::>(); + if registered.is_empty() { + return Ok(BTreeSet::new()); + } + + // The parent Git index, not HEAD or .gitmodules alone, must name a gitlink. + // New and staged submodules remain eligible before the parent's commit. + let mut command = source_git_command(project_root)?; + command.args(["--literal-pathspecs", "ls-files", "--stage", "-z", "--"]); + for path in ®istered { + command.arg(path); + } + let index = checked_source_git_output(&mut command)?; + let gitlinks = index + .split(|byte| *byte == 0) + .filter_map(|record| { + let separator = record.iter().position(|byte| *byte == b'\t')?; + let fields = std::str::from_utf8(&record[..separator]).ok()?; + let mut parts = fields.split_whitespace(); + if parts.next() != Some("160000") + || parts.next()?.len() != 40 + || parts.next() != Some("0") + || parts.next().is_some() + { + return None; + } + let path = std::str::from_utf8(&record[separator + 1..]).ok()?; + registered.contains(path).then(|| path.to_owned()) + }) + .collect::>(); + if gitlinks.is_empty() { + return Ok(BTreeSet::new()); + } + + let canonical_root = project_root.canonicalize().map_err(git_walk_error)?; + let mut accepted = BTreeSet::new(); + for path in gitlinks { + let checked_out = project_root.join(&path); + let marker = match std::fs::symlink_metadata(checked_out.join(".git")) { + Ok(metadata) => metadata.file_type().is_file() || metadata.file_type().is_dir(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(error) => return Err(git_walk_error(error)), + }; + if !marker { + continue; + } + let mut command = source_git_command(project_root)?; + command.arg("-C").arg(&path); + command.args(["rev-parse", "--show-superproject-working-tree"]); + let output = checked_source_git_output(&mut command)?; + let parent = std::str::from_utf8(&output) + .map_err(git_walk_error)? + .trim(); + if !parent.is_empty() + && Path::new(parent).canonicalize().map_err(git_walk_error)? == canonical_root + { + accepted.insert(PathBuf::from(path)); + } + } + Ok(accepted) +} + +fn git_walk_error(error: impl std::fmt::Display) -> SourceWalkError { + SourceWalkError { + glob: ".gitmodules".to_owned(), + message: format!("registered submodule discovery failed: {error}"), + } +} + +fn source_git_command(root: &Path) -> Result { + let program = tracedecay_runtime_core::git::try_git_program().map_err(git_walk_error)?; + let mut command = std::process::Command::new(program); + command.current_dir(root); + // Do not let ambient Git variables redirect the registered index or root. + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("GIT_") { + command.env_remove(key); + } + } + Ok(command) +} + +fn checked_source_git_output( + command: &mut std::process::Command, +) -> Result, SourceWalkError> { + let output = command.output().map_err(git_walk_error)?; + if !output.status.success() { + return Err(git_walk_error(String::from_utf8_lossy(&output.stderr))); + } + Ok(output.stdout) +} + fn build_overrides( project_root: &Path, path_glob: Option<&str>, @@ -375,6 +531,72 @@ mod tests { ); } + fn run_git(root: &std::path::Path, args: &[&str]) { + let status = std::process::Command::new( + tracedecay_runtime_core::git::try_git_program().expect("git executable"), + ) + .current_dir(root) + .args(args) + .status() + .expect("run Git"); + assert!(status.success(), "Git command failed: {args:?}"); + } + + #[test] + fn only_an_explicitly_included_registered_submodule_joins_the_parent_walk() { + let root = TempDir::new().expect("parent"); + let child = TempDir::new().expect("submodule source"); + run_git(root.path(), &["init", "-q"]); + run_git(child.path(), &["init", "-q"]); + fs::create_dir_all(child.path().join("src")).unwrap(); + fs::write(child.path().join("src/feature.rs"), "pub fn feature() {}\n").unwrap(); + run_git(child.path(), &["add", "."]); + run_git( + child.path(), + &[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "commit", + "-qm", + "initial child", + ], + ); + let git = tracedecay_runtime_core::git::try_git_program().expect("Git"); + let status = std::process::Command::new(git) + .current_dir(root.path()) + .args(["-c", "protocol.file.allow=always", "submodule", "add", "-q"]) + .arg(child.path()) + .arg("app") + .status() + .expect("add submodule"); + assert!(status.success()); + fs::create_dir_all(root.path().join("foreign/src")).unwrap(); + fs::create_dir(root.path().join("foreign/.git")).unwrap(); + fs::write(root.path().join("foreign/src/intruder.rs"), "foreign").unwrap(); + // A .gitmodules declaration without a gitlink is not authority. + fs::write( + root.path().join(".gitmodules"), + "[submodule \"app\"]\n\tpath = app\n\turl = ../child\n[submodule \"foreign\"]\n\tpath = foreign\n\turl = ../child\n", + ) + .unwrap(); + + let default = IndexPathPolicyV1::new(vec![], vec![]).unwrap(); + assert!( + !walked_files(root.path(), &default).contains(&PathBuf::from("app/src/feature.rs")), + "the default must still prune all nested Git repositories" + ); + let policy = IndexPathPolicyV1::new( + vec![], + vec!["app/**".into(), "foreign/**".into()], + ) + .unwrap(); + let files = walked_files(root.path(), &policy); + assert!(files.contains(&PathBuf::from("app/src/feature.rs")), "{files:?}"); + assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); + } + #[test] fn explicit_glob_cannot_read_excluded_paths_without_policy_include() { let root = TempDir::new().expect("project root"); From d2de6986cf3696bf18696ec126ec8b2e203c8582 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:27:54 -0800 Subject: [PATCH 2/5] fix(index): honor precise includes inside registered submodules Use the canonical compiled include policy to identify reachable submodule paths, including a single file or glob below the gitlink. Restrict file admission inside opted-in submodules to matches of those explicit includes; a broad traversal may not silently expose unrelated source files. Keep the normal exclude policy and authenticated gitlink/superproject checks. Add domain policy and real Git-submodule walker regressions. Rust/Bazel and full integration validation remain for hosted CI; production scheduler candidate enumeration is a separate known blocker before merge. --- .../tracedecay-code-index/src/source_walk.rs | 43 ++++++++++++++++--- .../src/source_path_policy.rs | 39 +++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/crates/tracedecay-code-index/src/source_walk.rs b/crates/tracedecay-code-index/src/source_walk.rs index 944fbf18e0..a587fe4d20 100644 --- a/crates/tracedecay-code-index/src/source_walk.rs +++ b/crates/tracedecay-code-index/src/source_walk.rs @@ -164,6 +164,22 @@ pub fn source_walk( } else { &path_policy }; + // Explicitly admitted submodules are otherwise broader than their + // include rules: an include is normally just an exclude override. + // Keep traversal confined to reachable directories, and admit + // child files only through the same compiled include authority. + if included_submodules + .iter() + .any(|submodule| Path::new(&relative).starts_with(submodule)) + { + if is_dir { + if !path_policy.include_can_reach(&relative) { + return false; + } + } else if !path_policy.explicitly_includes(&relative) { + return false; + } + } if !is_dir { return !path_policy.excludes(&relative); } @@ -250,11 +266,10 @@ fn included_git_submodules( { return None; } - let explicitly_included = policy.include_patterns().iter().any(|rule| { - let rule = rule.trim_start_matches("./").trim_start_matches('/'); - let rule = rule.trim_end_matches('/'); - rule == path || rule.strip_suffix("/**") == Some(path) - }); + // A selected file or glob below the gitlink also authorizes + // traversal to reach it; the per-entry matcher still filters + // unrelated child files. Reuse the compiled policy's reachability. + let explicitly_included = policy.include_can_reach(path); explicitly_included.then(|| path.to_owned()) }) .collect::>(); @@ -595,6 +610,24 @@ mod tests { let files = walked_files(root.path(), &policy); assert!(files.contains(&PathBuf::from("app/src/feature.rs")), "{files:?}"); assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); + + // A precise file include must both reach the registered submodule + // and refuse other child files. The unrestricted ordinary-file policy + // is not permission to index the whole submodule. + fs::write(root.path().join("app/src/unselected.rs"), "not selected\n").unwrap(); + for rule in ["app/src/feature.rs", "app/src/fe*.rs"] { + let narrow = IndexPathPolicyV1::new(vec![], vec![rule.to_owned()]).unwrap(); + let files = walked_files(root.path(), &narrow); + assert!( + files.contains(&PathBuf::from("app/src/feature.rs")), + "{rule}: the include must reach the child through its gitlink: {files:?}" + ); + assert!( + !files.contains(&PathBuf::from("app/src/unselected.rs")), + "{rule}: a narrow include must not admit unrelated files: {files:?}" + ); + assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); + } } #[test] diff --git a/crates/tracedecay-domain/src/source_path_policy.rs b/crates/tracedecay-domain/src/source_path_policy.rs index 9871687d7e..3dcb002525 100644 --- a/crates/tracedecay-domain/src/source_path_policy.rs +++ b/crates/tracedecay-domain/src/source_path_policy.rs @@ -140,6 +140,28 @@ impl IndexPathPolicyV1 { &self.include } + /// Whether a path, or one of its parents, is explicitly selected by an + /// index.include.v1 rule. Unlike excludes(), this does not treat an + /// unrestricted ordinary file as an explicit submodule opt-in. + #[must_use] + pub fn explicitly_includes(&self, logical_path: &str) -> bool { + matches_path_or_parent(&self.include_set, logical_path) + } + + /// Whether any compiled include may select a child of this directory. + /// A wildcard before the first literal segment conservatively admits + /// traversal; the canonical compiled matcher still decides actual files. + #[must_use] + pub fn include_can_reach(&self, logical_directory: &str) -> bool { + let directory = std::path::Path::new(logical_directory); + self.include_prefixes.iter().any(|prefix| { + prefix.as_deref().is_none_or(|prefix| { + let prefix = std::path::Path::new(prefix); + prefix.starts_with(directory) || directory.starts_with(prefix) + }) + }) + } + /// Whether `logical_path` is left out of the index and source walks. #[must_use] pub fn excludes(&self, logical_path: &str) -> bool { @@ -228,6 +250,23 @@ mod tests { .expect("valid patterns") } + #[test] + fn explicit_include_reachability_keeps_narrow_submodule_scopes() { + let precise = policy(&[], &["app/src/feature.rs"]); + assert!(precise.include_can_reach("app")); + assert!(precise.include_can_reach("app/src")); + assert!(precise.include_can_reach("app/src/feature.rs")); + assert!(!precise.include_can_reach("foreign")); + assert!(precise.explicitly_includes("app/src/feature.rs")); + assert!(!precise.explicitly_includes("app/src/other.rs")); + assert!(!precise.explicitly_includes("app")); + + let wildcard = policy(&[], &["**/src/*.rs"]); + assert!(wildcard.include_can_reach("app")); + assert!(wildcard.explicitly_includes("app/src/feature.rs")); + assert!(!wildcard.explicitly_includes("app/docs/other.md")); + } + #[test] fn exclude_matches_paths_and_their_parent_directories() { let policy = policy( From 7da327efed51b6dc734d4f255cd495d41abe3831 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:36:10 -0800 Subject: [PATCH 3/5] fix(index): preserve Git object formats and submodule root paths Let Git validate object IDs instead of assuming SHA-1 width, and remove only its output newline rather than whitespace belonging to a root path. Reuse one real Git fixture for SHA-1, SHA-256 and Unix whitespace roots; keep narrow includes, unrelated-clone rejection and removed-gitlink checks. Fail the walk fixture on entry errors instead of silently dropping them. Native Git mechanism probes pass. Rust/Bazel execution remains pending. --- .../tracedecay-code-index/src/source_walk.rs | 86 +++++++++++++------ 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/crates/tracedecay-code-index/src/source_walk.rs b/crates/tracedecay-code-index/src/source_walk.rs index a587fe4d20..e3f79ef847 100644 --- a/crates/tracedecay-code-index/src/source_walk.rs +++ b/crates/tracedecay-code-index/src/source_walk.rs @@ -291,8 +291,9 @@ fn included_git_submodules( let separator = record.iter().position(|byte| *byte == b'\t')?; let fields = std::str::from_utf8(&record[..separator]).ok()?; let mut parts = fields.split_whitespace(); + // Git validates the object ID using the repository's object format. if parts.next() != Some("160000") - || parts.next()?.len() != 40 + || parts.next().is_none() || parts.next() != Some("0") || parts.next().is_some() { @@ -322,9 +323,9 @@ fn included_git_submodules( command.arg("-C").arg(&path); command.args(["rev-parse", "--show-superproject-working-tree"]); let output = checked_source_git_output(&mut command)?; - let parent = std::str::from_utf8(&output) - .map_err(git_walk_error)? - .trim(); + let parent = std::str::from_utf8(&output).map_err(git_walk_error)?; + // Remove Git's line terminator, not whitespace belonging to the path. + let parent = parent.strip_suffix('\n').unwrap_or(parent); if !parent.is_empty() && Path::new(parent).canonicalize().map_err(git_walk_error)? == canonical_root { @@ -397,7 +398,7 @@ mod tests { fn walked_files(root: &std::path::Path, path_policy: &IndexPathPolicyV1) -> Vec { let mut files = source_walk(root, None, path_policy) .expect("source walk") - .filter_map(Result::ok) + .map(|entry| entry.expect("source entry")) .filter(|entry| entry.file_type().is_some_and(|kind| kind.is_file())) .map(|entry| { entry @@ -547,22 +548,19 @@ mod tests { } fn run_git(root: &std::path::Path, args: &[&str]) { - let status = std::process::Command::new( - tracedecay_runtime_core::git::try_git_program().expect("git executable"), - ) - .current_dir(root) - .args(args) - .status() - .expect("run Git"); + let status = super::source_git_command(root) + .expect("Git command") + .args(args) + .status() + .expect("run Git"); assert!(status.success(), "Git command failed: {args:?}"); } - #[test] - fn only_an_explicitly_included_registered_submodule_joins_the_parent_walk() { - let root = TempDir::new().expect("parent"); + fn assert_registered_submodule_walk(root: &std::path::Path, object_format: &str) { let child = TempDir::new().expect("submodule source"); - run_git(root.path(), &["init", "-q"]); - run_git(child.path(), &["init", "-q"]); + let format = format!("--object-format={object_format}"); + run_git(root, &["init", "-q", &format]); + run_git(child.path(), &["init", "-q", &format]); fs::create_dir_all(child.path().join("src")).unwrap(); fs::write(child.path().join("src/feature.rs"), "pub fn feature() {}\n").unwrap(); run_git(child.path(), &["add", "."]); @@ -578,28 +576,33 @@ mod tests { "initial child", ], ); - let git = tracedecay_runtime_core::git::try_git_program().expect("Git"); - let status = std::process::Command::new(git) - .current_dir(root.path()) + let status = super::source_git_command(root) + .expect("Git command") .args(["-c", "protocol.file.allow=always", "submodule", "add", "-q"]) .arg(child.path()) .arg("app") .status() .expect("add submodule"); assert!(status.success()); - fs::create_dir_all(root.path().join("foreign/src")).unwrap(); - fs::create_dir(root.path().join("foreign/.git")).unwrap(); - fs::write(root.path().join("foreign/src/intruder.rs"), "foreign").unwrap(); + let status = super::source_git_command(root) + .expect("Git command") + .args(["clone", "-q"]) + .arg(child.path()) + .arg("foreign") + .status() + .expect("clone unrelated repository"); + assert!(status.success()); + fs::write(root.join("foreign/src/intruder.rs"), "foreign").unwrap(); // A .gitmodules declaration without a gitlink is not authority. fs::write( - root.path().join(".gitmodules"), + root.join(".gitmodules"), "[submodule \"app\"]\n\tpath = app\n\turl = ../child\n[submodule \"foreign\"]\n\tpath = foreign\n\turl = ../child\n", ) .unwrap(); let default = IndexPathPolicyV1::new(vec![], vec![]).unwrap(); assert!( - !walked_files(root.path(), &default).contains(&PathBuf::from("app/src/feature.rs")), + !walked_files(root, &default).contains(&PathBuf::from("app/src/feature.rs")), "the default must still prune all nested Git repositories" ); let policy = IndexPathPolicyV1::new( @@ -607,17 +610,17 @@ mod tests { vec!["app/**".into(), "foreign/**".into()], ) .unwrap(); - let files = walked_files(root.path(), &policy); + let files = walked_files(root, &policy); assert!(files.contains(&PathBuf::from("app/src/feature.rs")), "{files:?}"); assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); // A precise file include must both reach the registered submodule // and refuse other child files. The unrestricted ordinary-file policy // is not permission to index the whole submodule. - fs::write(root.path().join("app/src/unselected.rs"), "not selected\n").unwrap(); + fs::write(root.join("app/src/unselected.rs"), "not selected\n").unwrap(); for rule in ["app/src/feature.rs", "app/src/fe*.rs"] { let narrow = IndexPathPolicyV1::new(vec![], vec![rule.to_owned()]).unwrap(); - let files = walked_files(root.path(), &narrow); + let files = walked_files(root, &narrow); assert!( files.contains(&PathBuf::from("app/src/feature.rs")), "{rule}: the include must reach the child through its gitlink: {files:?}" @@ -628,6 +631,33 @@ mod tests { ); assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); } + + run_git(root, &["update-index", "--force-remove", "app"]); + assert!( + !walked_files(root, &policy).contains(&PathBuf::from("app/src/feature.rs")), + "registration and a checked-out child cannot replace the parent gitlink" + ); + } + + #[test] + fn only_an_explicitly_included_registered_submodule_joins_the_parent_walk() { + let root = TempDir::new().expect("parent"); + assert_registered_submodule_walk(root.path(), "sha1"); + } + + #[test] + fn registered_submodules_support_sha256_object_ids() { + let root = TempDir::new().expect("SHA-256 parent"); + assert_registered_submodule_walk(root.path(), "sha256"); + } + + #[cfg(unix)] + #[test] + fn registered_submodules_preserve_whitespace_in_the_superproject_path() { + let temporary = TempDir::new().expect("parent directory"); + let root = temporary.path().join("project \t"); + fs::create_dir(&root).expect("whitespace-bearing project root"); + assert_registered_submodule_walk(&root, "sha1"); } #[test] From 0d0e71876e60a0431003280168ad1e8ce329fe66 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:54:42 -0800 Subject: [PATCH 4/5] fix(index): withdraw incomplete submodule source admission Keep the established repository boundary: a path include and parent gitlink do not provide child capture, freshness or indexed retrieval. Remove discovery subprocesses and the now-unused path-policy APIs instead of exposing files only to live search. Retain strict walk fixture errors. Add real registered-submodule coverage for broad, exact and glob includes with and without explicit search scope, and a regression for malformed metadata breaking unrelated source reads. Native Git fixture and failure probes pass. Rust/Bazel tests, formatting and the complete native matrix have not run in this tool-limited session. This withdraws the partial feature; it does not complete issue #4593. --- .../tracedecay-code-index/src/source_walk.rs | 362 +++++------------- .../src/source_path_policy.rs | 39 -- 2 files changed, 86 insertions(+), 315 deletions(-) diff --git a/crates/tracedecay-code-index/src/source_walk.rs b/crates/tracedecay-code-index/src/source_walk.rs index e3f79ef847..90bb4838ac 100644 --- a/crates/tracedecay-code-index/src/source_walk.rs +++ b/crates/tracedecay-code-index/src/source_walk.rs @@ -9,8 +9,7 @@ //! walk is therefore public rather than crate-private, the audit in the root //! crate reuses this policy instead of restating it. -use std::collections::BTreeSet; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::sync::Arc; use ignore::overrides::{Override, OverrideBuilder}; @@ -117,7 +116,6 @@ pub fn source_walk( let generated_dir_overrides = overrides.clone(); let path_glob_scope = path_glob.and_then(PathGlobScope::from_path_glob); let filter_root = project_root.to_path_buf(); - let included_submodules = included_git_submodules(project_root, path_policy)?; let path_policy = path_policy.clone(); let mut builder = WalkBuilder::new(project_root); @@ -164,32 +162,15 @@ pub fn source_walk( } else { &path_policy }; - // Explicitly admitted submodules are otherwise broader than their - // include rules: an include is normally just an exclude override. - // Keep traversal confined to reachable directories, and admit - // child files only through the same compiled include authority. - if included_submodules - .iter() - .any(|submodule| Path::new(&relative).starts_with(submodule)) - { - if is_dir { - if !path_policy.include_can_reach(&relative) { - return false; - } - } else if !path_policy.explicitly_includes(&relative) { - return false; - } - } if !is_dir { return !path_policy.excludes(&relative); } - // Nested Git repositories are independent sources except for - // authenticated submodules selected by an explicit index include. - // Linked worktrees and unrelated clones remain pruned even when - // a broad source glob would otherwise include their contents. - if std::fs::symlink_metadata(entry.path().join(".git")).is_ok() - && !included_submodules.contains(Path::new(&relative)) - { + // Includes select paths, not repository authority. A parent + // gitlink does not capture the child's files or prove their + // freshness, so even registered submodules remain separate + // sources. Linked worktrees and nested clones share this boundary. + // The project root is deliberately exempt at depth zero above. + if std::fs::symlink_metadata(entry.path().join(".git")).is_ok() { return false; } !path_policy.excludes_directory(&relative) @@ -211,160 +192,6 @@ pub fn forward_slash_relative(relative: &Path) -> Arc { tracedecay_domain::forward_slash_path(relative).into() } -/// A nested repository is part of this source tree only when an explicit -/// index.include.v1 path selects a registered, checked-out Git submodule. -/// Linked worktrees and unrelated clones also carry .git files, so the -/// marker alone never authorizes traversing their contents. -fn included_git_submodules( - project_root: &Path, - policy: &IndexPathPolicyV1, -) -> Result, SourceWalkError> { - if policy.include_patterns().is_empty() { - return Ok(BTreeSet::new()); - } - let modules_file = project_root.join(".gitmodules"); - match std::fs::symlink_metadata(&modules_file) { - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - return Ok(BTreeSet::new()); - } - Ok(metadata) if metadata.file_type().is_file() => {} - Ok(_) => return Err(git_walk_error(".gitmodules is not a regular file")), - Err(error) => return Err(git_walk_error(error)), - } - - let mut command = source_git_command(project_root)?; - command.args([ - "config", - "--null", - "--file", - ".gitmodules", - "--get-regexp", - r"^submodule\..*\.path$", - ]); - let modules = command.output().map_err(git_walk_error)?; - // Git config returns exit 1 when there are no matching keys. - if modules.status.code() == Some(1) { - return Ok(BTreeSet::new()); - } - if !modules.status.success() { - return Err(git_walk_error(String::from_utf8_lossy(&modules.stderr))); - } - let registered = modules - .stdout - .split(|byte| *byte == 0) - .filter_map(|record| { - let separator = record.iter().position(|byte| *byte == b'\n')?; - let key = std::str::from_utf8(&record[..separator]).ok()?; - if !key.starts_with("submodule.") || !key.ends_with(".path") { - return None; - } - let path = std::str::from_utf8(&record[separator + 1..]).ok()?; - let relative = Path::new(path); - if path.is_empty() - || relative.is_absolute() - || !relative.components().all(|part| matches!(part, Component::Normal(_))) - { - return None; - } - // A selected file or glob below the gitlink also authorizes - // traversal to reach it; the per-entry matcher still filters - // unrelated child files. Reuse the compiled policy's reachability. - let explicitly_included = policy.include_can_reach(path); - explicitly_included.then(|| path.to_owned()) - }) - .collect::>(); - if registered.is_empty() { - return Ok(BTreeSet::new()); - } - - // The parent Git index, not HEAD or .gitmodules alone, must name a gitlink. - // New and staged submodules remain eligible before the parent's commit. - let mut command = source_git_command(project_root)?; - command.args(["--literal-pathspecs", "ls-files", "--stage", "-z", "--"]); - for path in ®istered { - command.arg(path); - } - let index = checked_source_git_output(&mut command)?; - let gitlinks = index - .split(|byte| *byte == 0) - .filter_map(|record| { - let separator = record.iter().position(|byte| *byte == b'\t')?; - let fields = std::str::from_utf8(&record[..separator]).ok()?; - let mut parts = fields.split_whitespace(); - // Git validates the object ID using the repository's object format. - if parts.next() != Some("160000") - || parts.next().is_none() - || parts.next() != Some("0") - || parts.next().is_some() - { - return None; - } - let path = std::str::from_utf8(&record[separator + 1..]).ok()?; - registered.contains(path).then(|| path.to_owned()) - }) - .collect::>(); - if gitlinks.is_empty() { - return Ok(BTreeSet::new()); - } - - let canonical_root = project_root.canonicalize().map_err(git_walk_error)?; - let mut accepted = BTreeSet::new(); - for path in gitlinks { - let checked_out = project_root.join(&path); - let marker = match std::fs::symlink_metadata(checked_out.join(".git")) { - Ok(metadata) => metadata.file_type().is_file() || metadata.file_type().is_dir(), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, - Err(error) => return Err(git_walk_error(error)), - }; - if !marker { - continue; - } - let mut command = source_git_command(project_root)?; - command.arg("-C").arg(&path); - command.args(["rev-parse", "--show-superproject-working-tree"]); - let output = checked_source_git_output(&mut command)?; - let parent = std::str::from_utf8(&output).map_err(git_walk_error)?; - // Remove Git's line terminator, not whitespace belonging to the path. - let parent = parent.strip_suffix('\n').unwrap_or(parent); - if !parent.is_empty() - && Path::new(parent).canonicalize().map_err(git_walk_error)? == canonical_root - { - accepted.insert(PathBuf::from(path)); - } - } - Ok(accepted) -} - -fn git_walk_error(error: impl std::fmt::Display) -> SourceWalkError { - SourceWalkError { - glob: ".gitmodules".to_owned(), - message: format!("registered submodule discovery failed: {error}"), - } -} - -fn source_git_command(root: &Path) -> Result { - let program = tracedecay_runtime_core::git::try_git_program().map_err(git_walk_error)?; - let mut command = std::process::Command::new(program); - command.current_dir(root); - // Do not let ambient Git variables redirect the registered index or root. - for (key, _) in std::env::vars_os() { - if key.to_string_lossy().starts_with("GIT_") { - command.env_remove(key); - } - } - Ok(command) -} - -fn checked_source_git_output( - command: &mut std::process::Command, -) -> Result, SourceWalkError> { - let output = command.output().map_err(git_walk_error)?; - if !output.status.success() { - return Err(git_walk_error(String::from_utf8_lossy(&output.stderr))); - } - Ok(output.stdout) -} - fn build_overrides( project_root: &Path, path_glob: Option<&str>, @@ -547,117 +374,100 @@ mod tests { ); } - fn run_git(root: &std::path::Path, args: &[&str]) { - let status = super::source_git_command(root) - .expect("Git command") - .args(args) - .status() - .expect("run Git"); - assert!(status.success(), "Git command failed: {args:?}"); - } - - fn assert_registered_submodule_walk(root: &std::path::Path, object_format: &str) { - let child = TempDir::new().expect("submodule source"); - let format = format!("--object-format={object_format}"); - run_git(root, &["init", "-q", &format]); - run_git(child.path(), &["init", "-q", &format]); - fs::create_dir_all(child.path().join("src")).unwrap(); - fs::write(child.path().join("src/feature.rs"), "pub fn feature() {}\n").unwrap(); - run_git(child.path(), &["add", "."]); - run_git( - child.path(), - &[ + fn fixture_git(root: &std::path::Path, args: &[&str]) -> std::process::Output { + let program = tracedecay_runtime_core::git::try_git_program().expect("Git program"); + let mut command = std::process::Command::new(program); + command.current_dir(root); + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("GIT_") { + command.env_remove(key); + } + } + let output = command + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", root.join(".fixture-gitconfig")) + .args([ "-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid", - "commit", - "-qm", - "initial child", - ], - ); - let status = super::source_git_command(root) - .expect("Git command") - .args(["-c", "protocol.file.allow=always", "submodule", "add", "-q"]) - .arg(child.path()) - .arg("app") - .status() - .expect("add submodule"); - assert!(status.success()); - let status = super::source_git_command(root) - .expect("Git command") - .args(["clone", "-q"]) - .arg(child.path()) - .arg("foreign") - .status() - .expect("clone unrelated repository"); - assert!(status.success()); - fs::write(root.join("foreign/src/intruder.rs"), "foreign").unwrap(); - // A .gitmodules declaration without a gitlink is not authority. - fs::write( - root.join(".gitmodules"), - "[submodule \"app\"]\n\tpath = app\n\turl = ../child\n[submodule \"foreign\"]\n\tpath = foreign\n\turl = ../child\n", - ) - .unwrap(); - - let default = IndexPathPolicyV1::new(vec![], vec![]).unwrap(); + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .output() + .expect("run fixture Git"); assert!( - !walked_files(root, &default).contains(&PathBuf::from("app/src/feature.rs")), - "the default must still prune all nested Git repositories" + output.status.success(), + "Git {args:?}: {}", + String::from_utf8_lossy(&output.stderr) ); - let policy = IndexPathPolicyV1::new( - vec![], - vec!["app/**".into(), "foreign/**".into()], - ) - .unwrap(); - let files = walked_files(root, &policy); - assert!(files.contains(&PathBuf::from("app/src/feature.rs")), "{files:?}"); - assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); + output + } - // A precise file include must both reach the registered submodule - // and refuse other child files. The unrestricted ordinary-file policy - // is not permission to index the whole submodule. - fs::write(root.join("app/src/unselected.rs"), "not selected\n").unwrap(); - for rule in ["app/src/feature.rs", "app/src/fe*.rs"] { - let narrow = IndexPathPolicyV1::new(vec![], vec![rule.to_owned()]).unwrap(); - let files = walked_files(root, &narrow); + #[test] + fn includes_cannot_admit_registered_submodule_files_as_parent_source() { + let fixture = TempDir::new().expect("repository fixture"); + let root = fixture.path().join("parent"); + let child = fixture.path().join("child"); + fs::create_dir_all(root.join("src")).unwrap(); + fs::create_dir_all(child.join("src")).unwrap(); + fixture_git(&root, &["init", "-q"]); + fixture_git(&child, &["init", "-q"]); + fs::write(root.join("src/parent.rs"), "pub fn parent() {}\n").unwrap(); + fs::write(child.join("src/feature.rs"), "pub fn feature() {}\n").unwrap(); + fixture_git(&child, &["add", "."]); + fixture_git(&child, &["commit", "-qm", "child source"]); + fixture_git( + &root, + &[ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + "-q", + "../child", + "app", + ], + ); + let index = fixture_git(&root, &["ls-files", "--stage", "-z", "--", "app"]); + assert!(index.stdout.starts_with(b"160000 ")); + assert!(index.stdout.ends_with(b" 0\tapp\0")); + assert!(root.join("app/src/feature.rs").is_file()); + + for rule in ["app/**", "app/src/feature.rs", "app/src/fe*.rs"] { + let policy = IndexPathPolicyV1::new(vec![], vec![rule.into()]).unwrap(); + let files = walked_files(&root, &policy); + assert!(files.contains(&PathBuf::from("src/parent.rs")), "{files:?}"); assert!( - files.contains(&PathBuf::from("app/src/feature.rs")), - "{rule}: the include must reach the child through its gitlink: {files:?}" + files.iter().all(|path| !path.starts_with("app")), + "{rule}: a parent gitlink is not child source authority: {files:?}" ); + let scoped = source_walk(&root, Some(rule), &policy) + .expect("explicit submodule scope") + .map(|entry| entry.expect("source entry")) + .filter(|entry| entry.file_type().is_some_and(|kind| kind.is_file())) + .collect::>(); assert!( - !files.contains(&PathBuf::from("app/src/unselected.rs")), - "{rule}: a narrow include must not admit unrelated files: {files:?}" + scoped.is_empty(), + "{rule}: an explicit scope cannot cross repositories" ); - assert!(!files.contains(&PathBuf::from("foreign/src/intruder.rs"))); } - - run_git(root, &["update-index", "--force-remove", "app"]); - assert!( - !walked_files(root, &policy).contains(&PathBuf::from("app/src/feature.rs")), - "registration and a checked-out child cannot replace the parent gitlink" - ); - } - - #[test] - fn only_an_explicitly_included_registered_submodule_joins_the_parent_walk() { - let root = TempDir::new().expect("parent"); - assert_registered_submodule_walk(root.path(), "sha1"); } #[test] - fn registered_submodules_support_sha256_object_ids() { - let root = TempDir::new().expect("SHA-256 parent"); - assert_registered_submodule_walk(root.path(), "sha256"); - } - - #[cfg(unix)] - #[test] - fn registered_submodules_preserve_whitespace_in_the_superproject_path() { - let temporary = TempDir::new().expect("parent directory"); - let root = temporary.path().join("project \t"); - fs::create_dir(&root).expect("whitespace-bearing project root"); - assert_registered_submodule_walk(&root, "sha1"); + fn submodule_metadata_cannot_break_an_unrelated_source_include() { + let root = TempDir::new().expect("project root"); + fs::create_dir_all(root.path().join(".git")).unwrap(); + fs::create_dir_all(root.path().join("src")).unwrap(); + fs::create_dir_all(root.path().join("app/.git")).unwrap(); + fs::write(root.path().join("src/keep.rs"), "pub fn keep() {}\n").unwrap(); + fs::write(root.path().join("app/foreign.rs"), "pub fn foreign() {}\n").unwrap(); + fs::write(root.path().join(".gitmodules"), "[invalid submodule metadata\n").unwrap(); + let policy = IndexPathPolicyV1::new(vec![], vec!["src/**".into()]).unwrap(); + let files = walked_files(root.path(), &policy); + assert!(files.contains(&PathBuf::from("src/keep.rs"))); + assert!(files.iter().all(|path| !path.starts_with("app")), "{files:?}"); } #[test] diff --git a/crates/tracedecay-domain/src/source_path_policy.rs b/crates/tracedecay-domain/src/source_path_policy.rs index 3dcb002525..9871687d7e 100644 --- a/crates/tracedecay-domain/src/source_path_policy.rs +++ b/crates/tracedecay-domain/src/source_path_policy.rs @@ -140,28 +140,6 @@ impl IndexPathPolicyV1 { &self.include } - /// Whether a path, or one of its parents, is explicitly selected by an - /// index.include.v1 rule. Unlike excludes(), this does not treat an - /// unrestricted ordinary file as an explicit submodule opt-in. - #[must_use] - pub fn explicitly_includes(&self, logical_path: &str) -> bool { - matches_path_or_parent(&self.include_set, logical_path) - } - - /// Whether any compiled include may select a child of this directory. - /// A wildcard before the first literal segment conservatively admits - /// traversal; the canonical compiled matcher still decides actual files. - #[must_use] - pub fn include_can_reach(&self, logical_directory: &str) -> bool { - let directory = std::path::Path::new(logical_directory); - self.include_prefixes.iter().any(|prefix| { - prefix.as_deref().is_none_or(|prefix| { - let prefix = std::path::Path::new(prefix); - prefix.starts_with(directory) || directory.starts_with(prefix) - }) - }) - } - /// Whether `logical_path` is left out of the index and source walks. #[must_use] pub fn excludes(&self, logical_path: &str) -> bool { @@ -250,23 +228,6 @@ mod tests { .expect("valid patterns") } - #[test] - fn explicit_include_reachability_keeps_narrow_submodule_scopes() { - let precise = policy(&[], &["app/src/feature.rs"]); - assert!(precise.include_can_reach("app")); - assert!(precise.include_can_reach("app/src")); - assert!(precise.include_can_reach("app/src/feature.rs")); - assert!(!precise.include_can_reach("foreign")); - assert!(precise.explicitly_includes("app/src/feature.rs")); - assert!(!precise.explicitly_includes("app/src/other.rs")); - assert!(!precise.explicitly_includes("app")); - - let wildcard = policy(&[], &["**/src/*.rs"]); - assert!(wildcard.include_can_reach("app")); - assert!(wildcard.explicitly_includes("app/src/feature.rs")); - assert!(!wildcard.explicitly_includes("app/docs/other.md")); - } - #[test] fn exclude_matches_paths_and_their_parent_directories() { let policy = policy( From e59a83c451d11dc0e50387e1369d8d3a55836e74 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 04:30:15 -0800 Subject: [PATCH 5/5] style: rustfmt the registered submodule source walk Co-Authored-By: Claude Opus 5.5 --- crates/tracedecay-code-index/src/source_walk.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/tracedecay-code-index/src/source_walk.rs b/crates/tracedecay-code-index/src/source_walk.rs index 90bb4838ac..63d4be6e40 100644 --- a/crates/tracedecay-code-index/src/source_walk.rs +++ b/crates/tracedecay-code-index/src/source_walk.rs @@ -463,11 +463,18 @@ mod tests { fs::create_dir_all(root.path().join("app/.git")).unwrap(); fs::write(root.path().join("src/keep.rs"), "pub fn keep() {}\n").unwrap(); fs::write(root.path().join("app/foreign.rs"), "pub fn foreign() {}\n").unwrap(); - fs::write(root.path().join(".gitmodules"), "[invalid submodule metadata\n").unwrap(); + fs::write( + root.path().join(".gitmodules"), + "[invalid submodule metadata\n", + ) + .unwrap(); let policy = IndexPathPolicyV1::new(vec![], vec!["src/**".into()]).unwrap(); let files = walked_files(root.path(), &policy); assert!(files.contains(&PathBuf::from("src/keep.rs"))); - assert!(files.iter().all(|path| !path.starts_with("app")), "{files:?}"); + assert!( + files.iter().all(|path| !path.starts_with("app")), + "{files:?}" + ); } #[test]