From 5b23ff896046014917a0fe2a0c9c041bc6e8e65a Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 7 Oct 2026 10:37:32 +0100 Subject: [PATCH 1/9] fix(grep,mcp_server): refuse fff-search mmap content cache at every FilePicker call site (Refs #293) fff-search 0.8.4 maps file content through `memmap2::Mmap::map` exactly once, in `FileItem::get_cached_content` (`src/types.rs:640`). That call's safety contract requires the mapped file not be modified concurrently, and the crate delegates the guarantee to a background watcher -- but `FilePicker::new` never installs one (`background_watcher: None`; the `watch` field is only consulted by `new_with_shared_state`), and all four call sites in this workspace use `FilePicker::new`. Every workspace grep therefore mapped files with no invalidation. Observed consequences (UF-039, CONFIRMED_UB by experiment EXP-104): with a concurrent same-length writer, 440 invalid-UTF-8 `line_content` strings out of 186M lines; and an in-place truncation during a grep kills the process with SIGBUS -- EXP-302 killed the shipped `terraphim_mcp_server --sse` with exit 135. The remedy is an explicit `ContentCacheBudget` whose `max_files` is 0, so `get_cached_content` returns `None` at its count guard before reaching `absolute_path` or `Mmap::map`, and `get_content_for_search` falls through to the slow path which reads each file into an owned buffer. Owned bytes cannot be mutated by another process, so both arms of the finding are gone. On Windows `get_cached_content` is already a stub returning `None`, so this makes Unix behave as Windows already does. `max_file_size` deliberately stays at 10 MiB. It also gates the slow path -- `get_content_for_search` rejects any file whose `size > max_file_size` -- so `ContentCacheBudget::zero()` or `max_file_size: 0` would silence grep entirely while looking like a working fix. `from_overrides` cannot express `max_files: 0` either (it reads 0 as "library default"), hence the struct literal. The value is hoisted into one constant per crate and shared with `GrepSearchOptions` so the two cannot drift. `follow_symlinks: false` is now stated at all four sites rather than inherited from `FilePickerOptions::default()`, so reasoning about which entries get indexed rests on an asserted value. `terraphim_mcp_server` does not depend on `terraphim_grep`, so the helper is duplicated rather than shared; coupling two binaries for eight lines is not worth it. A new `ci_guards` test walks `crates/**/*.rs` with `std::fs` (the Gitea runner's program allowlist rejects anything that is not `cargo` or a repo script) and asserts no `cache_budget: None` remains and that every `FilePicker::new` names `no_mmap_budget()` and `follow_symlinks: false` in its options block. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjGks4DGYdDw3QLgu32S96 --- crates/terraphim_agent/tests/ci_guards.rs | 118 +++++++++++++++++++ crates/terraphim_grep/src/hybrid_searcher.rs | 54 ++++++++- crates/terraphim_mcp_server/src/lib.rs | 76 +++++++++++- 3 files changed, 241 insertions(+), 7 deletions(-) diff --git a/crates/terraphim_agent/tests/ci_guards.rs b/crates/terraphim_agent/tests/ci_guards.rs index 8cca9655..f0429fab 100644 --- a/crates/terraphim_agent/tests/ci_guards.rs +++ b/crates/terraphim_agent/tests/ci_guards.rs @@ -654,3 +654,121 @@ fn publish_gate_tests_pass() { String::from_utf8_lossy(&out.stderr), ); } + +/// Collect every `.rs` file under `crates/`, skipping build directories. +/// +/// Uses `std::fs` rather than shelling out to `fd`/`rg`: the Gitea runner's +/// program allowlist rejects anything that is not `cargo` or a repo script, and +/// a guard that silently cannot run is worse than no guard. +fn workspace_rust_sources() -> Vec { + fn walk(dir: &Path, out: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + let Ok(file_type) = entry.file_type() else { + continue; + }; + if file_type.is_dir() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + // Build output and vendored sources are not this repo's code. + if name.starts_with("target") || name == ".git" { + continue; + } + walk(&path, out); + } else if file_type.is_file() && path.extension().is_some_and(|e| e == "rs") { + out.push(path); + } + } + } + + let mut out = Vec::new(); + walk(&workspace_root().join("crates"), &mut out); + // This guard file quotes the very literals it forbids, so exclude it. + let own_name = Path::new(file!()) + .file_name() + .expect("guard file name") + .to_owned(); + out.retain(|p| p.file_name() != Some(own_name.as_os_str())); + out.sort(); + out +} + +/// Every `FilePicker::new` call must refuse fff-search's mmap content cache. +/// +/// fff-search's only `memmap2::Mmap::map` call sits in `get_cached_content`, and +/// its safety contract requires that the mapped file is not modified +/// concurrently. The crate delegates that guarantee to a background watcher +/// which `FilePicker::new` never installs, so a concurrent writer produces +/// invalid-UTF-8 match text and an in-place truncation raises SIGBUS (EXP-302 +/// killed `terraphim_mcp_server --sse` with exit 135 this way). The remedy is a +/// budget whose `max_files` is 0, which makes `get_cached_content` bail at its +/// count guard before it maps anything. +/// +/// Leaving `cache_budget` at `None` -- or omitting it, which `Default` turns +/// into `None` -- asks fff-search to size a budget from the repo and reopens the +/// finding. Hence: no `cache_budget: None` anywhere, and every picker must name +/// `no_mmap_budget()` and `follow_symlinks: false` in its options block. +/// +/// See UF-039 / #293. +#[test] +fn every_file_picker_refuses_the_mmap_cache() { + // Built by concatenation so this test's own source does not contain the + // literal it forbids (the walk excludes this file too, belt and braces). + let forbidden = concat!("cache_budget: ", "None"); + + for path in workspace_rust_sources() { + let Ok(text) = std::fs::read_to_string(&path) else { + continue; + }; + let display = path.display(); + + for (i, line) in text.lines().enumerate() { + assert!( + !line.contains(forbidden), + "{display}:{} relies on fff-search sizing its own mmap cache budget; \ + pass `Some(no_mmap_budget())` instead (UF-039 / #293)", + i + 1, + ); + } + + let lines: Vec<&str> = text.lines().collect(); + for (i, line) in lines.iter().enumerate() { + if !line.contains("FilePicker::new(") { + continue; + } + // The options block runs from the call to the struct-update tail. + let end = lines[i..] + .iter() + .position(|l| l.contains("..FilePickerOptions::default()")) + .map(|off| i + off) + .unwrap_or_else(|| { + panic!( + "{display}:{} opens a FilePicker without a \ + `..FilePickerOptions::default()` tail; this guard cannot \ + establish what its options are", + i + 1 + ) + }); + let block = lines[i..=end].join("\n"); + + assert!( + block.contains("no_mmap_budget()"), + "{display}:{} opens a FilePicker without `no_mmap_budget()`; \ + fff-search would then mmap file content with no invalidation \ + (UF-039 / #293)", + i + 1, + ); + assert!( + block.contains("follow_symlinks: false"), + "{display}:{} opens a FilePicker without an explicit \ + `follow_symlinks: false`; the guard reasoning assumes symlinks \ + are not followed and must not rest on an upstream default \ + (Refs #293)", + i + 1, + ); + } + } +} diff --git a/crates/terraphim_grep/src/hybrid_searcher.rs b/crates/terraphim_grep/src/hybrid_searcher.rs index 4b0cd219..1fec0036 100644 --- a/crates/terraphim_grep/src/hybrid_searcher.rs +++ b/crates/terraphim_grep/src/hybrid_searcher.rs @@ -215,6 +215,50 @@ pub fn boost_chunks_with_kg( chunks } +/// Upper bound on the size of a file `search_code` will read. +/// +/// Shared between the content-cache budget and `GrepSearchOptions` so the two +/// cannot drift apart: a file large enough to be refused by one but not the +/// other would be a silent inconsistency. +#[cfg(feature = "code-search")] +const MAX_GREP_FILE_SIZE: u64 = 10 * 1024 * 1024; + +/// A content-cache budget that refuses fff-search's mmap content cache outright. +/// +/// fff-search maps file content through `memmap2::Mmap::map` (the crate's only +/// such call, in `get_cached_content`), whose safety contract requires that the +/// mapped file is not modified concurrently. The crate delegates that guarantee +/// to a background watcher, but `FilePicker::new` never installs one, so a +/// concurrent writer yields invalid-UTF-8 match text and an in-place truncation +/// raises SIGBUS. `max_files: 0` makes `get_cached_content` return `None` at its +/// count guard before it reaches `absolute_path` or `Mmap::map`, so every read +/// takes the slow path and lands in an owned buffer that no other process can +/// mutate. On Windows `get_cached_content` is already a stub returning `None`, +/// so this simply makes Unix behave the same way. See UF-039 / #293. +/// +/// `max_file_size` must stay non-zero. It also gates the *slow* path +/// (`get_content_for_search` rejects any file whose `size > max_file_size`), so +/// zeroing it -- or using `ContentCacheBudget::zero()` -- would silence grep +/// entirely rather than harden it: zero matches, looking like a working fix. +/// `ContentCacheBudget::from_overrides` cannot express this budget either, as it +/// reads `max_files: 0` as "use the library default", hence the struct literal. +/// +/// Scope: this removes *fff-search's* mapping, not all mapping in the process. +/// `FrecencyTracker::open` still memory-maps an LMDB data file and libgit2 maps +/// pack files; neither is covered here. +#[cfg(feature = "code-search")] +fn no_mmap_budget() -> fff_search::ContentCacheBudget { + use std::sync::atomic::{AtomicU64, AtomicUsize}; + + fff_search::ContentCacheBudget { + max_files: 0, + max_bytes: 0, + max_file_size: MAX_GREP_FILE_SIZE, + cached_count: AtomicUsize::new(0), + cached_bytes: AtomicU64::new(0), + } +} + pub struct HybridSearcher { role_graph: Arc>, /// Kept alongside the rolegraph so KG-style boosting still works when no documents @@ -355,7 +399,13 @@ impl HybridSearcher { base_path: search_path.to_string_lossy().to_string(), mode: FFFMode::Ai, watch: false, - cache_budget: None, + // See `no_mmap_budget` -- UF-039 / #293. + cache_budget: Some(no_mmap_budget()), + // Asserted rather than inherited from `FilePickerOptions::default()` + // so that reasoning about which entries fff-search will index rests + // on a value this crate states, not on an upstream default that a + // minor release could change. Refs #293. + follow_symlinks: false, ..FilePickerOptions::default() }) .map_err(|e| format!("FilePicker init failed: {}", e))?; @@ -370,7 +420,7 @@ impl HybridSearcher { let fff_query = parse_grep_query(query); let options = GrepSearchOptions { - max_file_size: 10 * 1024 * 1024, + max_file_size: MAX_GREP_FILE_SIZE, max_matches_per_file: 200, smart_case: true, file_offset: 0, diff --git a/crates/terraphim_mcp_server/src/lib.rs b/crates/terraphim_mcp_server/src/lib.rs index 1f169fd0..b4a35455 100644 --- a/crates/terraphim_mcp_server/src/lib.rs +++ b/crates/terraphim_mcp_server/src/lib.rs @@ -91,6 +91,54 @@ impl From for ErrorData { } } +/// Upper bound on the size of a file the grep tools will read. +/// +/// Shared between the content-cache budget and `GrepSearchOptions` so the two +/// cannot drift apart: a file large enough to be refused by one but not the +/// other would be a silent inconsistency. +const MAX_GREP_FILE_SIZE: u64 = 10 * 1024 * 1024; + +/// A content-cache budget that refuses fff-search's mmap content cache outright. +/// +/// fff-search maps file content through `memmap2::Mmap::map` (the crate's only +/// such call, in `get_cached_content`), whose safety contract requires that the +/// mapped file is not modified concurrently. The crate delegates that guarantee +/// to a background watcher, but `FilePicker::new` never installs one, so a +/// concurrent writer yields invalid-UTF-8 match text and an in-place truncation +/// raises SIGBUS -- which killed the shipped `terraphim_mcp_server --sse` with +/// exit 135 during the audit. `max_files: 0` makes `get_cached_content` return +/// `None` at its count guard before it reaches `absolute_path` or `Mmap::map`, +/// so every read takes the slow path and lands in an owned buffer that no other +/// process can mutate. On Windows `get_cached_content` is already a stub +/// returning `None`, so this simply makes Unix behave the same way. +/// See UF-039 / #293. +/// +/// `max_file_size` must stay non-zero. It also gates the *slow* path +/// (`get_content_for_search` rejects any file whose `size > max_file_size`), so +/// zeroing it -- or using `ContentCacheBudget::zero()` -- would silence grep +/// entirely rather than harden it: zero matches, looking like a working fix. +/// `ContentCacheBudget::from_overrides` cannot express this budget either, as it +/// reads `max_files: 0` as "use the library default", hence the struct literal. +/// +/// Scope: this removes *fff-search's* mapping, not all mapping in the process. +/// `FrecencyTracker::open` still memory-maps an LMDB data file and libgit2 maps +/// pack files; neither is covered here. +/// +/// This duplicates `terraphim_grep`'s helper deliberately: this crate does not +/// depend on `terraphim_grep`, and adding a dependency to share eight lines +/// would couple two binaries for no benefit. +fn no_mmap_budget() -> fff_search::ContentCacheBudget { + use std::sync::atomic::{AtomicU64, AtomicUsize}; + + fff_search::ContentCacheBudget { + max_files: 0, + max_bytes: 0, + max_file_size: MAX_GREP_FILE_SIZE, + cached_count: AtomicUsize::new(0), + cached_bytes: AtomicU64::new(0), + } +} + /// The main service type for the Terraphim MCP server #[derive(Clone)] pub struct McpService { @@ -1275,7 +1323,13 @@ impl McpService { base_path: base_path.clone(), mode: FFFMode::Ai, watch: false, - cache_budget: None, + // See `no_mmap_budget` -- UF-039 / #293. + cache_budget: Some(no_mmap_budget()), + // Asserted rather than inherited from `FilePickerOptions::default()` + // so that reasoning about which entries fff-search will index rests + // on a value this crate states, not on an upstream default that a + // minor release could change. Refs #293. + follow_symlinks: false, ..FilePickerOptions::default() }) .map_err(|e| ErrorData::internal_error(format!("FilePicker init failed: {e}"), None))?; @@ -1373,7 +1427,13 @@ impl McpService { base_path: base_path.clone(), mode: FFFMode::Ai, watch: false, - cache_budget: None, + // See `no_mmap_budget` -- UF-039 / #293. + cache_budget: Some(no_mmap_budget()), + // Asserted rather than inherited from `FilePickerOptions::default()` + // so that reasoning about which entries fff-search will index rests + // on a value this crate states, not on an upstream default that a + // minor release could change. Refs #293. + follow_symlinks: false, ..FilePickerOptions::default() }) .map_err(|e| ErrorData::internal_error(format!("FilePicker init failed: {e}"), None))?; @@ -1390,7 +1450,7 @@ impl McpService { let fff_query = parse_grep_query(&query); let options = GrepSearchOptions { - max_file_size: 10 * 1024 * 1024, + max_file_size: MAX_GREP_FILE_SIZE, max_matches_per_file: 200, smart_case: true, file_offset, @@ -1486,7 +1546,13 @@ impl McpService { base_path: base_path.clone(), mode: FFFMode::Ai, watch: false, - cache_budget: None, + // See `no_mmap_budget` -- UF-039 / #293. + cache_budget: Some(no_mmap_budget()), + // Asserted rather than inherited from `FilePickerOptions::default()` + // so that reasoning about which entries fff-search will index rests + // on a value this crate states, not on an upstream default that a + // minor release could change. Refs #293. + follow_symlinks: false, ..FilePickerOptions::default() }) .map_err(|e| ErrorData::internal_error(format!("FilePicker init failed: {e}"), None))?; @@ -1509,7 +1575,7 @@ impl McpService { let parsed_constraints = parsed.constraints.as_slice(); let options = GrepSearchOptions { - max_file_size: 10 * 1024 * 1024, + max_file_size: MAX_GREP_FILE_SIZE, max_matches_per_file: 200, smart_case: true, file_offset, From 4305bcaf72debebf281d022469adc029f76a1ce4 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 7 Oct 2026 11:27:15 +0100 Subject: [PATCH 2/9] fix(grep,mcp_server): refuse trees with non-UTF-8 or over-long entry names before fff-search indexes them (Refs #296) fff-search stores every relative path as `to_string_lossy().into_owned()`. On Linux each invalid byte in an entry name becomes a three-byte U+FFFD, so a hostile name inflates threefold. `ChunkedString::read_to_buf` (`src/simd_path.rs:170-186`) then copies `byte_len.min(buf.len())` bytes into a 4096-byte buffer and returns `core::str::from_utf8_unchecked(&buf[..total])` -- so the cut lands inside a replacement character and the returned `&str` is not valid UTF-8. The path runs through `FileItem::absolute_path` (`src/types.rs:271-275`) from `get_content_for_search` and from `collect_files`' git-status lookup, so it is taken for every file. UF-040, CONFIRMED_UB by experiment EXP-201: invalid-UTF-8 `PathBuf`/`str` values in five of seven hostile geometries, plus `DirItem::dir_name` panicking with "start byte index 10724 is out of bounds for string of length 4096". The base path is attacker-chosen through `terraphim-grep --paths` and the MCP tools' `path` argument. This is not covered by the zero-file cache budget from #293: that budget stops `get_cached_content` before it maps, but `get_content_for_search` then falls through to the slow path and still calls `absolute_path` on every file, and `collect_files` calls it before any grep runs. fff-search 0.8.4 offers no path-length cap, no strict-UTF-8 mode and no non-lossy storage switch -- all three verified absent from `FilePickerOptions`' seven fields -- so the remedy has to sit in front of the library. A pre-walk rejects the tree when an entry name is not valid UTF-8, or when a path relative to the search root exceeds `PATH_BUF_SIZE - 1` bytes. The second rule is a deliberate conservative extension covering deep trees of *valid* multi-byte names, which truncate mid-character by the same mechanism; EXP-201 did not test that geometry, so it is reasoned from `read_to_buf` rather than observed. The walk reads names only -- `DirEntry::file_type` uses the directory entry's type, with no `stat` -- and does not descend into symbolic links, matching the `follow_symlinks: false` the pickers now assert. An unreadable directory is skipped rather than reported, so an ordinary permission quirk does not become a refused search; `find_files_invalid_path_returns_error` still exercises fff-search's own message for a nonexistent base path. Placed as `terraphim_grep::path_guard::reject_hostile_tree`, called at the head of `HybridSearcher::search_code` before `FilePicker::new` (covering the CLI and every library caller), and as a private copy in `terraphim_mcp_server` called at the head of `find_files`, `grep_files` and `multi_grep_files`, surfacing as `ErrorData::invalid_params`. The duplication is deliberate: `terraphim_mcp_server` does not depend on `terraphim_grep`. Residual risks, documented at both copies: a TOCTOU window remains, since an entry created between the pre-walk and `collect_files` still reaches the inflation -- that needs an active concurrent writer inside the searched tree, whereas the confirmed exposure is a static hostile checkout. The walk also does not honour `.gitignore`, so `target/` and `node_modules/` are walked; both callers already pay a full `collect_files` scan immediately afterwards. Scope note, also recorded in the code: the #293 budget removes *fff-search's* mapping, not all mapping in the process. `FrecencyTracker::open` still memory-maps an LMDB data file and libgit2 maps pack files. Neither commit should be read as "this process no longer mmaps". Tests use real directories, no mocks. The hostile cases build six nested directories each named 250 raw `0xFF` bytes via `std::os::unix::ffi::OsStrExt` (1500 raw bytes, 4500 once inflated) and assert both `reject_hostile_tree` and the production entry points refuse them, naming the offending entry. They carry `#[cfg_attr(not(target_os = "linux"), ignore = ...)]` rather than `#[cfg]` so their bodies still compile and lint on this workspace's macOS hosts -- APFS refuses non-UTF-8 entry names, so they cannot run there. `plain_tree_still_greps` is the control that matters: it plants a match in an ordinary tree and asserts grep returns it end-to-end through `HybridSearcher::search`. That is what distinguishes the #293 remedy from the trap of zeroing `max_file_size`, which would also close the slow read path and return zero matches while looking like a working fix. The `ci_guards` arm now also requires a `reject_hostile_tree(` call within the 40 lines preceding every `FilePicker::new(` under `crates/`. The per-entry decision is split into `check_entry` in both copies so the two rules can be tested without asking the host filesystem for a geometry it refuses. That matters for rule (b): macOS caps a path at 1024 bytes and Linux at 4096, and `std::fs` passes absolute paths, so no tree whose *relative* path exceeds 4095 bytes can be created through it -- the first attempt at a filesystem-based test failed with ENAMETOOLONG. Rule (b) is therefore a backstop for a future fff-search that stores paths differently, not something a real tree reaches today, and the unit tests say so. Rule (a) is unit-tested through `check_entry` as well, which gives it coverage on macOS, where the integration test cannot run. `plain_tree_still_greps` also plants the needle in a 64 KiB file, not only a small one. `get_cached_content` returns `None` for anything below `MMAP_THRESHOLD` (16 KiB on aarch64, 4 KiB elsewhere) *before* it consults the budget, so a small file takes the slow path with or without the #293 fix and a control built only on one proves nothing. The large file is the geometry the zero-file budget actually diverts: threshold guard passes, `0 >= max_files` returns `None` without mapping, slow path reads it into an owned buffer, needle still matches. Setting `MAX_GREP_FILE_SIZE` to 0 was confirmed to turn this test red, so it does catch the trap it is there for. The MCP test asserts on `ErrorData::message` rather than `Display`: it can never run on macOS, so a wrong guess about rmcp's formatting would surface only as a red Linux CI. The length rule is applied to the base path and the relative path together, because fff-search spends them from one budget. `FileItem::write_absolute_path` (`src/types.rs:337-357`) writes the base path, a separator and the relative path into a single `[u8; PATH_BUF_SIZE]`, lets `read_to_buf` truncate the relative portion to whatever space is left, and returns the result through `from_utf8_unchecked`. Two consequences follow, and the first version of this guard, which measured only the relative portion, missed both: - A long search root leaves little room for the relative portion, so the truncation cuts a valid multi-byte character in half. No hostile entry name is needed; EXP-201's geometry is sufficient but not necessary. This is reachable on Linux with ordinary names -- a root of roughly 3000 bytes and relative paths of roughly 1200 each keep every absolute path inside `PATH_MAX` while their sum clears the 4096-byte buffer. - A root over 4096 bytes makes `buf[..base_len].copy_from_slice(base)` panic outright rather than truncate, which is why that case gets its own error text. So `base_prefix_len` now reproduces `write_absolute_path`'s own arithmetic (the root's encoded bytes plus one separator unless it already ends in `MAIN_SEPARATOR`), a root at or over the buffer is refused before the walk begins, and the per-entry rule is `base_end_idx + rel_len >= PATH_BUF_SIZE`. The error apportions the two contributions, because a user whose relative paths are plainly short would otherwise find the refusal baffling, and suggests searching from a shorter root. `>=` rather than `>` keeps the byte `write_relative_cstr` reserves for its NUL terminator (`types.rs:372`). The reachability note in the earlier revision of this branch -- that the length rule was a backstop no real tree could reach, because `std::fs` passes absolute paths and `PATH_MAX` is 1024 on macOS and 4096 on Linux -- was wrong for exactly this reason, and is retracted. It holds only for the relative portion in isolation. Four unit tests cover the arithmetic, and unlike the hostile-tree integration tests they run on macOS: `base_prefix_len` against the library's `sep_len` logic, an over-long root refused before any `read_dir` (so no unbuildable tree is needed), a long root with short ordinary ASCII names refused, and the `PATH_BUF_SIZE - 1` boundary accepted with one byte more refused. A search root whose own path is not valid UTF-8 is refused before anything else, because otherwise the length arithmetic above measures a different string from the one fff-search receives. `terraphim_grep`'s call site passes `base_path: search_path.to_string_lossy().to_string()`, which replaces each invalid byte with a three-byte U+FFFD, so such a root is stored up to three times longer than `base_prefix_len` counted -- enough to let an overflowing tree through, and enough for the inflated root alone to panic `buf[..base_len].copy_from_slice(base)` with no relative path involved. The root is never an entry the walk discovers, so the per-entry UTF-8 rule did not cover it. Refusing is the right answer irrespective of the arithmetic: the lossily stored name no longer refers to the directory on disk, so fff-search could not open anything beneath it. That is the same silent-drop defect the audit recorded for hostile entry names, and a named error beats searching and finding nothing. Reachable through `terraphim-grep --paths` and `HybridSearcher::with_search_paths`, since clap builds `Vec` from `OsString` and invalid bytes survive on Linux. The MCP tools are immune -- their `path` arrives as JSON and is therefore already valid UTF-8, and the `String` handed to fff-search is byte for byte the one the guard checked -- but both copies carry the check so neither depends on where it was called from. `base_prefix_len`'s doc records that its raw-byte count matches what fff-search receives only because this check has already run. An audit of both call sites confirms `base_path` is the only path-ish input field of `FilePickerOptions`, and that no other differently-encoded copy of a path reaches the library: `args.paths` passes through to `search_path` unchanged, and the `relative_path` values in the MCP tools are outputs. Two further unit tests, both running on macOS: a non-UTF-8 root is refused (needing no on-disk tree, since the check precedes any `read_dir`), and a valid root containing Cyrillic, Japanese and multi-byte Latin characters is still accepted -- the check rejects an invalid root, not a non-ASCII one, and the reverse would be a real regression for users with non-Latin paths. A note on the `>=` in the length rule, since an earlier revision of this branch justified it wrongly and the wrong reason invites a future maintainer to loosen the rule while "fixing" the comment. `write_absolute_path`'s exact truncation threshold is `base_end_idx + rel_len > PATH_BUF_SIZE`: the relative portion is read into `&mut buf[base_end_idx..]` and `read_to_buf` takes `byte_len.min(buf.len())`, so a total of exactly `PATH_BUF_SIZE` fits and nothing is cut. The NUL byte `write_relative_cstr` reserves comes out of that function's own buffer (`types.rs:366-375`) and does not shrink `write_absolute_path`'s, so the earlier claim that `>=` was needed to preserve it was simply incorrect. `>=` is kept regardless, deliberately. One rule then also covers `write_relative_cstr`, whose threshold is `rel_len >= PATH_BUF_SIZE`, and which `base_end_idx >= 0` makes a subset of this condition -- no second branch needed. It also keeps the guard off the exact dependency boundary, which would otherwise have to be re-derived on every fff-search upgrade. The cost is that an absolute path summing to exactly `PATH_BUF_SIZE` bytes is refused although `write_absolute_path` alone would handle it; that over-rejection is intended, and both the code comment and `total_length_boundary_is_exact`'s doc now say so, the latter so the test is not read as pinning the library's own threshold. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjGks4DGYdDw3QLgu32S96 --- crates/terraphim_agent/tests/ci_guards.rs | 14 + crates/terraphim_grep/src/hybrid_searcher.rs | 5 + crates/terraphim_grep/src/lib.rs | 2 + crates/terraphim_grep/src/path_guard.rs | 448 ++++++++++++++++++ .../tests/path_guard_hostile_tree.rs | 201 ++++++++ crates/terraphim_mcp_server/src/lib.rs | 237 +++++++++ .../tests/test_find_files.rs | 63 +++ 7 files changed, 970 insertions(+) create mode 100644 crates/terraphim_grep/src/path_guard.rs create mode 100644 crates/terraphim_grep/tests/path_guard_hostile_tree.rs diff --git a/crates/terraphim_agent/tests/ci_guards.rs b/crates/terraphim_agent/tests/ci_guards.rs index f0429fab..407d826b 100644 --- a/crates/terraphim_agent/tests/ci_guards.rs +++ b/crates/terraphim_agent/tests/ci_guards.rs @@ -761,6 +761,20 @@ fn every_file_picker_refuses_the_mmap_cache() { (UF-039 / #293)", i + 1, ); + // fff-search stores relative paths lossily and then truncates them + // into a fixed 4096-byte buffer behind `from_utf8_unchecked`, so a + // non-UTF-8 or over-long entry name yields an invalid `&str` and can + // panic in `DirItem::dir_name`. The base path is attacker-chosen, so + // the tree must be refused before the picker sees it. UF-040 / #296. + let preceding = lines[i.saturating_sub(40)..i].join("\n"); + assert!( + preceding.contains("reject_hostile_tree("), + "{display}:{} opens a FilePicker without a `reject_hostile_tree(` \ + call in the preceding 40 lines; a hostile entry name would then \ + reach fff-search's `from_utf8_unchecked` (UF-040 / #296)", + i + 1, + ); + assert!( block.contains("follow_symlinks: false"), "{display}:{} opens a FilePicker without an explicit \ diff --git a/crates/terraphim_grep/src/hybrid_searcher.rs b/crates/terraphim_grep/src/hybrid_searcher.rs index 1fec0036..e8ccf376 100644 --- a/crates/terraphim_grep/src/hybrid_searcher.rs +++ b/crates/terraphim_grep/src/hybrid_searcher.rs @@ -395,6 +395,11 @@ impl HybridSearcher { parse_grep_query, }; + // Refuse the tree before its base path reaches fff-search: a + // non-UTF-8 or over-long entry name defeats the library's + // `from_utf8_unchecked` on the stored relative path. UF-040 / #296. + crate::path_guard::reject_hostile_tree(&search_path)?; + let mut picker = FilePicker::new(FilePickerOptions { base_path: search_path.to_string_lossy().to_string(), mode: FFFMode::Ai, diff --git a/crates/terraphim_grep/src/lib.rs b/crates/terraphim_grep/src/lib.rs index 3cd79945..ff2c44dd 100644 --- a/crates/terraphim_grep/src/lib.rs +++ b/crates/terraphim_grep/src/lib.rs @@ -25,6 +25,7 @@ pub mod hybrid_searcher; pub mod kg_curation; #[cfg(feature = "llm")] pub mod openrouter_client; +pub mod path_guard; pub mod rlm_context; pub mod signatures; pub mod sufficiency_judge; @@ -37,6 +38,7 @@ pub use hybrid_searcher::{ RetrievedChunk, boost_chunks_with_kg, score_kg_boost, }; pub use kg_curation::KgCurationRlm; +pub use path_guard::reject_hostile_tree; pub use rlm_context::RlmContext; pub use signatures::{AnswerWithCitations, Citation, Match, NewConcept, RlmSignature}; pub use sufficiency_judge::{ diff --git a/crates/terraphim_grep/src/path_guard.rs b/crates/terraphim_grep/src/path_guard.rs new file mode 100644 index 00000000..bce9b47e --- /dev/null +++ b/crates/terraphim_grep/src/path_guard.rs @@ -0,0 +1,448 @@ +//! Refuse a directory tree before fff-search indexes it. +//! +//! Two mechanisms converge on the same place, `FileItem::write_absolute_path` +//! (`fff-search-0.8.4/src/types.rs:337-357`). It writes the base path, a +//! separator and the relative path into ONE `[u8; PATH_BUF_SIZE]` buffer, lets +//! `ChunkedString::read_to_buf` truncate the relative portion to whatever space +//! is left, and returns the result through +//! `core::str::from_utf8_unchecked(&buf[..total])`. +//! +//! 1. **Lossy storage inflates hostile names.** fff-search stores every relative +//! path as `to_string_lossy().into_owned()`, so on Linux each invalid byte in +//! an entry name becomes a three-byte U+FFFD and the name inflates threefold. +//! The truncation then cuts inside a replacement character. +//! +//! 2. **The base path shares the budget.** Because base and relative are written +//! into the same 4096 bytes, a long base path leaves little room for the +//! relative portion, and the cut lands mid-character in ordinary valid +//! multi-byte names. No hostile name is needed: the entry names of EXP-201 +//! are sufficient but not necessary. A base path over 4096 bytes is worse +//! still -- `buf[..base_len].copy_from_slice(base)` panics outright. +//! +//! Either way the returned `&str` is not valid UTF-8. The call is reached from +//! `FileItem::absolute_path` and from `write_absolute_path`'s five callers +//! (`file_picker.rs:424`, `file_picker.rs:669`, `types.rs:428`, `types.rs:471`, +//! `scan.rs:408`), so both `get_content_for_search` and `collect_files`' +//! git-status lookup take it for every file. +//! +//! The base path is attacker-chosen: `terraphim-grep --paths` and the MCP +//! tools' `path` argument both hand it straight to fff-search. Upstream offers +//! no path-length cap, no strict-UTF-8 mode and no non-lossy storage switch, so +//! the remedy has to sit in front of the library. See UF-040 / #296. + +use std::ffi::OsStr; +use std::path::Path; + +/// Mirrors fff-search's `PATH_BUF_SIZE` (`simd_path.rs`). +/// +/// Not configurable upstream, so it is reproduced here. If a future fff-search +/// changes it, this guard becomes conservative rather than unsound. +const PATH_BUF_SIZE: usize = 4096; + +/// The number of bytes `write_absolute_path` consumes before the relative path. +/// +/// Mirrors that function's own arithmetic exactly (`types.rs:343-351`) so the +/// two cannot drift: the base path's encoded bytes, plus one separator unless +/// the base already ends in `MAIN_SEPARATOR`. +/// +/// This counts the root's *raw* bytes, while the call sites hand fff-search +/// `base_path: search_path.to_string_lossy().to_string()`. The two agree only +/// because `reject_hostile_tree` has already refused a root that is not valid +/// UTF-8, which makes `to_string_lossy` a no-op on it. That is the invariant the +/// whole length rule rests on: without it a root with invalid bytes would be +/// stored up to three times longer than measured here, and the arithmetic would +/// understate what the buffer actually holds. +fn base_prefix_len(base: &Path) -> usize { + let bytes = base.as_os_str().as_encoded_bytes(); + let base_len = bytes.len(); + let sep_len = + usize::from(base_len > 0 && bytes[base_len - 1] != std::path::MAIN_SEPARATOR as u8); + base_len + sep_len +} + +/// Decide one directory entry, returning its length relative to the search root. +/// +/// `base_end_idx` is what `base_prefix_len` returned for the search root: the +/// buffer is shared, so the length rule has to be applied to the total, not to +/// the relative portion alone. +/// +/// Split out from the walk so both rules can be exercised without asking the +/// host filesystem for a geometry it refuses to create -- macOS caps a single +/// pathname at 1024 bytes and Linux at 4096, so the interesting combinations of +/// a long base and a long relative path cannot always be built on disk even +/// though fff-search would mishandle them. +fn check_entry( + base: &Path, + dir: &Path, + name: &OsStr, + base_end_idx: usize, + parent_rel_len: usize, +) -> Result { + if name.to_str().is_none() { + return Err(format!( + "refusing to search {}: entry {:?} under {} has a name that is not \ + valid UTF-8. fff-search stores paths lossily, which inflates each \ + invalid byte threefold, and the shared {PATH_BUF_SIZE}-byte path \ + buffer then truncates inside a replacement character and returns \ + the result through `from_utf8_unchecked` (UF-040 / #296).", + base.display(), + name, + dir.display(), + )); + } + + // Relative length as fff-search stores it: the parent's relative length, a + // separator once there is a parent component, and the name. + let name_len = name.as_encoded_bytes().len(); + let rel_len = if parent_rel_len == 0 { + name_len + } else { + parent_rel_len + 1 + name_len + }; + + // `write_absolute_path` fills one buffer with base, separator and relative, + // so the budget is shared. + // + // Its exact truncation threshold is `base_end_idx + rel_len > PATH_BUF_SIZE`: + // the relative portion is read into `&mut buf[base_end_idx..]`, and + // `read_to_buf` takes `byte_len.min(buf.len())`, so a total of exactly + // `PATH_BUF_SIZE` fits and nothing is cut. This guard uses `>=`, one byte + // stricter, DELIBERATELY -- do not "correct" it to `>`: + // + // - One rule then also covers `write_relative_cstr`, whose own threshold is + // `rel_len >= PATH_BUF_SIZE` because it reserves its buffer's last byte for + // a NUL (`types.rs:366-375`). Since `base_end_idx >= 0`, this condition + // subsumes that one and no second branch is needed. + // - It keeps the guard off the exact dependency boundary, which would + // otherwise have to be re-derived on every fff-search upgrade. + // + // The cost is that an absolute path summing to exactly `PATH_BUF_SIZE` bytes + // is refused although `write_absolute_path` alone would handle it. That is + // intended. + if base_end_idx + rel_len >= PATH_BUF_SIZE { + return Err(format!( + "refusing to search {}: the absolute path to {:?} under {} would be \ + {} bytes ({base_end_idx} for the search root and its separator, plus \ + {rel_len} relative), at or over fff-search's {PATH_BUF_SIZE}-byte path \ + buffer. `write_absolute_path` writes both halves into that one buffer \ + and truncates whatever does not fit, then returns the result through \ + `from_utf8_unchecked` -- which cuts a multi-byte character in half. \ + Searching from a shorter root is usually enough to fix this \ + (UF-040 / #296).", + base.display(), + name, + dir.display(), + base_end_idx + rel_len, + )); + } + + Ok(rel_len) +} + +/// Reject a tree that would defeat fff-search's `from_utf8_unchecked`. +/// +/// Fails, naming the offending entry, when any of +/// +/// - the search root's own path is not valid UTF-8 -- it would be stored lossily, +/// inflating it and leaving a name that no longer refers to the directory; +/// - an entry name is not valid UTF-8 -- lossy storage inflates it threefold and +/// the truncation then cuts inside a replacement character; +/// - the search root alone fills `PATH_BUF_SIZE`, which makes +/// `write_absolute_path`'s `copy_from_slice` panic rather than truncate; or +/// - the root plus a relative path reaches `PATH_BUF_SIZE`. Base and relative +/// share one buffer (`types.rs:337-357`), so the budget is joint. +/// +/// The third rule is not merely a conservative extension. It covers ordinary +/// trees of *valid* names reached from a long root, where the truncation cuts a +/// multi-byte character in half with no hostile name involved -- the EXP-201 +/// geometry is sufficient but not necessary. EXP-201 did not test this +/// combination, so it is reasoned from `write_absolute_path` and `read_to_buf` +/// rather than observed. +/// +/// The walk reads names only -- `DirEntry::file_type` uses the directory +/// entry's type, with no `stat` -- and does not follow symbolic links, matching +/// the `follow_symlinks: false` the pickers now assert. An unreadable directory +/// is skipped rather than reported: fff-search will skip it too, and failing +/// here would turn an ordinary permission quirk into a refused search. +/// +/// Residual risk: a TOCTOU window remains. An entry created between this walk +/// and fff-search's `collect_files` still reaches the inflation. That needs an +/// active concurrent writer inside the searched tree, whereas the confirmed +/// exposure is a static hostile checkout. +/// +/// The walk does not honour `.gitignore`, so `target/` and `node_modules/` are +/// walked. For a CLI invocation and a per-call MCP picker that cost is +/// acceptable; both already pay a full `collect_files` scan immediately after. +pub fn reject_hostile_tree(base: &Path) -> Result<(), String> { + // fff-search receives `base_path` as a `String`. `terraphim_grep`'s call site + // builds it with `to_string_lossy()`, which replaces each invalid byte with a + // three-byte U+FFFD, so a root with invalid bytes would be stored up to three + // times longer than `base_prefix_len` measures -- defeating the arithmetic + // below, and panicking `copy_from_slice` outright once the inflated root + // alone reaches the buffer. Refusing such a root restores the invariant. + // + // It is the right answer regardless of the arithmetic: the lossily stored + // name no longer names the on-disk entry, so fff-search cannot open anything + // under it. That is the same silent-drop defect the audit recorded for + // hostile entry names, and a named error beats searching and finding nothing. + // + // Reachable through `terraphim-grep --paths` and `with_search_paths`, since + // clap builds `Vec` from `OsString` and invalid bytes survive on + // Linux. The MCP tools are immune -- their `path` arrives as JSON and is + // always valid UTF-8 -- but both copies carry the check so neither depends on + // where it was called from. + // + // The root is never an entry the walk discovers, so the per-entry UTF-8 rule + // in `check_entry` does not cover it. + if base.to_str().is_none() { + return Err(format!( + "refusing to search {:?}: the search root's own path is not valid \ + UTF-8. fff-search stores it lossily, which both inflates each invalid \ + byte threefold past the {PATH_BUF_SIZE}-byte path buffer and leaves a \ + stored name that no longer refers to the directory on disk, so no file \ + under it could be opened (UF-040 / #296).", + base.as_os_str(), + )); + } + + // `write_absolute_path` writes the base path into the same fixed buffer as + // the relative path, so the root's own length is part of the budget -- and a + // root at or over the buffer makes `copy_from_slice` panic outright. + let base_end_idx = base_prefix_len(base); + if base_end_idx >= PATH_BUF_SIZE { + return Err(format!( + "refusing to search {}: the search root alone is {base_end_idx} bytes \ + with its separator, at or over fff-search's {PATH_BUF_SIZE}-byte path \ + buffer, which leaves no room for any relative path. \ + `write_absolute_path` copies the root into that buffer unconditionally, \ + so a root this long panics the library rather than merely truncating \ + (UF-040 / #296).", + base.display(), + )); + } + + let mut stack = vec![(base.to_path_buf(), 0usize)]; + + while let Some((dir, rel_len)) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + + for entry in entries.flatten() { + let name = entry.file_name(); + let entry_rel_len = check_entry(base, &dir, &name, base_end_idx, rel_len)?; + + // `file_type` reports a symbolic link as such, so links are never + // descended into -- no `metadata` call, and no loop to detect. + if entry.file_type().is_ok_and(|t| t.is_dir()) { + stack.push((entry.path(), entry_rel_len)); + } + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn plain_tree_is_accepted() { + let tmp = tempfile::tempdir().expect("tempdir"); + std::fs::create_dir_all(tmp.path().join("src/inner")).expect("create dirs"); + std::fs::write(tmp.path().join("src/inner/lib.rs"), "fn main() {}\n").expect("write"); + + assert!(reject_hostile_tree(tmp.path()).is_ok()); + } + + /// `base_prefix_len` must mirror `write_absolute_path`'s `sep_len` exactly: + /// one separator is added unless the root already ends in one. + #[test] + fn base_prefix_len_matches_the_library_arithmetic() { + let sep = std::path::MAIN_SEPARATOR; + assert_eq!(base_prefix_len(Path::new("")), 0); + assert_eq!(base_prefix_len(Path::new(&format!("{sep}abc"))), 5); + assert_eq!(base_prefix_len(Path::new(&format!("{sep}abc{sep}"))), 5); + } + + /// Rule (a): a name that is not valid UTF-8 is refused, and the error shows + /// the offending bytes. + /// + /// Exercised through `check_entry` rather than a real tree because APFS + /// refuses to create such a name at all, so the filesystem would fail before + /// the guard was reached. The bytes here are the same ones the Linux-only + /// integration test plants on disk. + #[test] + #[cfg(unix)] + fn non_utf8_name_is_rejected() { + use std::os::unix::ffi::OsStrExt; + + let raw = vec![0xFFu8; 250]; + let name = OsStr::from_bytes(&raw); + + let err = check_entry(Path::new("/base"), Path::new("/base/sub"), name, 6, 0) + .expect_err("a non-UTF-8 name must be refused"); + assert!( + err.contains("not valid UTF-8"), + "error must say why, got: {err}" + ); + assert!( + err.contains("\\xff") || err.contains("\\xFF"), + "error must show the offending bytes, got: {err}" + ); + } + + /// A search root whose own path is not valid UTF-8 is refused. + /// + /// This is what keeps `base_prefix_len`'s arithmetic honest: the call site + /// hands fff-search `to_string_lossy()` of this path, which would be up to + /// three times longer than the raw bytes measured here. The root need not + /// exist -- the check runs before any `read_dir` -- which is what lets this + /// test run on macOS, where such a path cannot be created at all. + #[test] + #[cfg(unix)] + fn non_utf8_base_path_is_refused() { + use std::os::unix::ffi::OsStrExt; + + let sep = std::path::MAIN_SEPARATOR; + let mut raw = format!("{sep}tmp{sep}").into_bytes(); + raw.extend_from_slice(&[0xFFu8; 8]); + let base = std::path::PathBuf::from(OsStr::from_bytes(&raw)); + + let err = reject_hostile_tree(&base) + .expect_err("a search root that is not valid UTF-8 must be refused"); + assert!( + err.contains("search root's own path is not valid UTF-8"), + "error must blame the root's encoding, got: {err}" + ); + assert!( + err.contains("\\xff") || err.contains("\\xFF"), + "error must show the offending bytes, got: {err}" + ); + } + + /// A valid-UTF-8 root containing multi-byte characters is still accepted. + /// + /// The check above rejects an *invalid* root, not a non-ASCII one. Refusing + /// non-Latin paths would be a real functional regression, and is the obvious + /// way to get this wrong. + #[test] + fn multi_byte_utf8_base_path_is_accepted() { + let tmp = tempfile::tempdir().expect("tempdir"); + // Cyrillic, Japanese and an emoji: three, three and four bytes each. + let root = tmp.path().join("проект").join("検索").join("fo\u{00f6}"); + std::fs::create_dir_all(root.join("src")).expect("create multi-byte tree"); + std::fs::write(root.join("src/lib.rs"), "fn main() {}\n").expect("write"); + + assert!( + root.to_str().is_some(), + "the fixture root must itself be valid UTF-8" + ); + assert_eq!( + reject_hostile_tree(&root), + Ok(()), + "a valid multi-byte root must be accepted, not mistaken for a hostile one" + ); + } + + /// A search root that alone fills the buffer is refused before the walk. + /// + /// This is the panic arm: `write_absolute_path` copies the root into the + /// buffer unconditionally, so `copy_from_slice` aborts the process rather + /// than truncating. The root need not exist -- the check runs before any + /// `read_dir`, which is what lets this test run on macOS, where a pathname + /// this long cannot be created at all. + #[test] + fn over_long_base_path_is_refused() { + let sep = std::path::MAIN_SEPARATOR; + let base = format!("{sep}{}", "b".repeat(PATH_BUF_SIZE)); + + let err = reject_hostile_tree(Path::new(&base)) + .expect_err("a search root at or over the buffer must be refused"); + assert!( + err.contains("search root alone"), + "error must blame the root, not an entry, got: {err}" + ); + assert!( + err.contains("no room for any relative path"), + "error must explain the consequence, got: {err}" + ); + } + + /// A long root plus short, ordinary ASCII entry names is refused. + /// + /// This is the case the first version of this guard missed: the relative + /// path is plainly short, so a rule that looked only at the relative portion + /// let it through, yet `write_absolute_path` shares one buffer between the + /// two and truncates the relative half mid-character. + #[test] + fn long_base_with_short_entry_names_is_refused() { + let sep = std::path::MAIN_SEPARATOR; + // 40 bytes short of the buffer, so even an 8-byte name overflows it. + let base = format!("{sep}{}", "b".repeat(PATH_BUF_SIZE - 41)); + let base = Path::new(&base); + let base_end_idx = base_prefix_len(base); + assert!( + base_end_idx < PATH_BUF_SIZE, + "the root itself must be legal" + ); + + let err = check_entry( + base, + base, + OsStr::new("main.rs"), + base_end_idx, + // A parent directory of 40 ASCII bytes -- nothing unusual. + 40, + ) + .expect_err("a long root plus an ordinary name must be refused"); + + assert!( + err.contains("for the search root and its separator"), + "error must apportion the budget so the user can see the root is the \ + cause, got: {err}" + ); + assert!( + err.contains("main.rs"), + "error must name the offending entry, got: {err}" + ); + } + + /// Boundary: `base_end_idx + rel_len == PATH_BUF_SIZE - 1` is accepted, and + /// one byte more is refused. + /// + /// This pins the guard's rule, not the library's. `write_absolute_path` + /// would itself handle a total of exactly `PATH_BUF_SIZE` without + /// truncating; the guard refuses it by choice, one byte early, so that the + /// same rule covers `write_relative_cstr` too. See `check_entry`. + /// + /// No filesystem can present this geometry -- macOS caps a single pathname + /// at 1024 bytes and Linux at 4096, and `std::fs` passes absolute paths -- + /// so `check_entry` is the honest place to pin the arithmetic. + #[test] + fn total_length_boundary_is_exact() { + let base = Path::new("/base"); + let base_end_idx = 100; + let name = "a".repeat(10); + // 100 + parent + 1 + 10 == 4095 exactly. + let parent_rel_len = PATH_BUF_SIZE - 1 - base_end_idx - 1 - name.len(); + + let rel_len = check_entry(base, base, OsStr::new(&name), base_end_idx, parent_rel_len) + .expect("a total of exactly PATH_BUF_SIZE - 1 bytes must be accepted"); + assert_eq!(base_end_idx + rel_len, PATH_BUF_SIZE - 1); + + let err = check_entry( + base, + base, + OsStr::new(&name), + base_end_idx, + parent_rel_len + 1, + ) + .expect_err("one byte more must be refused"); + assert!( + err.contains("at or over fff-search's"), + "error must explain the length rule, got: {err}" + ); + } +} diff --git a/crates/terraphim_grep/tests/path_guard_hostile_tree.rs b/crates/terraphim_grep/tests/path_guard_hostile_tree.rs new file mode 100644 index 00000000..24032f24 --- /dev/null +++ b/crates/terraphim_grep/tests/path_guard_hostile_tree.rs @@ -0,0 +1,201 @@ +//! `HybridSearcher::search_code` must refuse a hostile tree before fff-search +//! indexes it, and must still grep an ordinary tree normally. +//! +//! fff-search stores relative paths lossily, so each invalid byte in an entry +//! name becomes a three-byte U+FFFD. `ChunkedString::read_to_buf` then copies +//! `byte_len.min(buf.len())` bytes into a 4096-byte buffer and hands the result +//! to `core::str::from_utf8_unchecked`, so an inflated path is cut inside a +//! replacement character and the returned `&str` is not valid UTF-8. EXP-201 +//! observed invalid-UTF-8 `PathBuf`/`str` values in five of seven hostile +//! geometries, plus `DirItem::dir_name` panicking with "start byte index 10724 +//! is out of bounds for string of length 4096". See UF-040 / #296. +//! +//! Real directories throughout -- no mocks. The hostile cases only run on +//! Linux: APFS rejects non-UTF-8 entry names outright, so `create_dir` fails on +//! macOS before the guard is reached. They are `ignore`d rather than `cfg`'d out +//! so that their bodies still compile and lint on this workspace's macOS hosts. + +use std::path::PathBuf; + +use terraphim_grep::{GrepOptions, Haystack, HybridSearcher, reject_hostile_tree}; +use terraphim_types::Thesaurus; + +/// Build a tree of `depth` nested directories whose names are `len` repeats of +/// `byte`, returning the deepest path actually created. +#[cfg(unix)] +fn nested_dirs(root: &std::path::Path, byte: u8, len: usize, depth: usize) -> PathBuf { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let raw = vec![byte; len]; + let segment = OsStr::from_bytes(&raw); + + let mut dir = root.to_path_buf(); + for _ in 0..depth { + dir = dir.join(segment); + std::fs::create_dir(&dir).expect("create nested directory"); + } + dir +} + +/// A tree of six nested directories each named 250 invalid bytes is refused, +/// and the error names the offending entry. +/// +/// 6 x 250 bytes is 1500 raw, which inflates to 4500 under lossy storage -- +/// comfortably past fff-search's 4096-byte path buffer. +#[test] +#[cfg_attr( + not(target_os = "linux"), + ignore = "APFS refuses non-UTF-8 entry names, so the tree cannot be built here" +)] +#[cfg(unix)] +fn hostile_non_utf8_tree_is_refused() { + let tmp = tempfile::tempdir().expect("tempdir"); + let deepest = nested_dirs(tmp.path(), 0xFF, 250, 6); + std::fs::write(deepest.join("payload.rs"), "fn needle() {}\n").expect("write payload"); + + let err = reject_hostile_tree(tmp.path()).expect_err("hostile tree must be refused"); + assert!( + err.contains("not valid UTF-8"), + "error must say why the tree was refused, got: {err}" + ); + assert!( + err.contains("\\xff") || err.contains("\\xFF"), + "error must name the offending entry's bytes, got: {err}" + ); +} + +/// The same tree must also be refused through the production entry point, not +/// only by the guard in isolation. +#[tokio::test] +#[cfg_attr( + not(target_os = "linux"), + ignore = "APFS refuses non-UTF-8 entry names, so the tree cannot be built here" +)] +#[cfg(unix)] +async fn hostile_non_utf8_tree_is_refused_by_search() { + let tmp = tempfile::tempdir().expect("tempdir"); + let deepest = nested_dirs(tmp.path(), 0xFF, 250, 6); + std::fs::write(deepest.join("payload.rs"), "fn needle() {}\n").expect("write payload"); + + let searcher = HybridSearcher::new("Guard Test".to_string(), Thesaurus::new("empty".into())) + .expect("HybridSearcher builds from an empty thesaurus") + .with_search_path(tmp.path().to_path_buf()); + + let err = searcher + .search( + "needle", + &GrepOptions { + haystack: Haystack::Code, + max_results: 5, + ..GrepOptions::default() + }, + ) + .await + .expect_err("search over a hostile tree must fail"); + + assert!( + err.contains("not valid UTF-8"), + "search must surface the guard's reason, got: {err}" + ); +} + +/// Files at or above fff-search's mmap threshold are exactly the ones the +/// zero-file budget diverts, so the needle must live in one. +/// +/// `MMAP_THRESHOLD` is 16 KiB on aarch64 and 4 KiB elsewhere, and +/// `get_cached_content` returns `None` for anything smaller *before* it consults +/// the budget. A small file would therefore take the slow path with or without +/// the fix, and a control built on one proves nothing. 64 KiB clears both +/// thresholds and stays far below the 10 MiB `max_file_size`. +const OVER_MMAP_THRESHOLD: usize = 64 * 1024; + +/// Control: an ordinary tree of valid ASCII names still greps normally, and +/// files large enough to have been mmapped still match. +/// +/// This is the arm that catches the trap in the UF-039 remedy. Zeroing +/// `max_file_size` -- or using `ContentCacheBudget::zero()` -- would also close +/// `get_content_for_search`'s slow path, so grep would return no matches at all +/// and the mmap fix would look like it worked. +/// +/// The large file is what makes the control meaningful. Its path through +/// fff-search is: `size >= MMAP_THRESHOLD`, so the threshold guard does not +/// bail; then `cached_count >= max_files` with `0 >= 0`, so `get_cached_content` +/// returns `None` without mapping; then the slow path reads it into an owned +/// buffer because `size <= max_file_size`; and the needle still matches. That is +/// precisely the behaviour #293 claims, and it is the assertion that would go +/// red for the right reason under a budget that also closed the slow path. +#[tokio::test] +async fn plain_tree_still_greps() { + let tmp = tempfile::tempdir().expect("tempdir"); + let nested = tmp.path().join("a".repeat(250)).join("b".repeat(250)); + std::fs::create_dir_all(&nested).expect("create control tree"); + + // Small file: exercises the threshold guard's early return. + std::fs::write( + nested.join("payload.rs"), + "fn guard_control_needle() { /* found */ }\n", + ) + .expect("write small payload"); + + // Large file: the geometry that used to be mmapped. + let filler = "// padding so this file clears the mmap threshold\n"; + let mut big = String::with_capacity(OVER_MMAP_THRESHOLD + 128); + while big.len() < OVER_MMAP_THRESHOLD { + big.push_str(filler); + } + big.push_str("fn guard_control_needle_large() { /* found */ }\n"); + let big_path = nested.join("big_payload.rs"); + std::fs::write(&big_path, &big).expect("write large payload"); + assert!( + std::fs::metadata(&big_path) + .expect("stat large payload") + .len() + >= OVER_MMAP_THRESHOLD as u64, + "the large payload must clear fff-search's mmap threshold for this control to mean anything" + ); + + assert!( + reject_hostile_tree(tmp.path()).is_ok(), + "a tree of valid ASCII names well inside the buffer must be accepted" + ); + + let searcher = HybridSearcher::new("Guard Test".to_string(), Thesaurus::new("empty".into())) + .expect("HybridSearcher builds from an empty thesaurus") + .with_search_path(tmp.path().to_path_buf()); + + let results = searcher + .search( + "guard_control_needle", + &GrepOptions { + haystack: Haystack::Code, + max_results: 50, + ..GrepOptions::default() + }, + ) + .await + .expect("search over an ordinary tree must succeed"); + + assert!( + results + .code_results + .iter() + .any(|c| c.content.contains("guard_control_needle()")), + "grep must still return the match in the small file; a zero-file cache \ + budget must not close the slow read path. Got {} results: {:?}", + results.code_results.len(), + results.code_results, + ); + + assert!( + results + .code_results + .iter() + .any(|c| c.content.contains("guard_control_needle_large()")), + "grep must still return the match in a file at or above fff-search's mmap \ + threshold -- that is the geometry the zero-file budget diverts to the slow \ + path, so this is the assertion #293 actually rests on. Got {} results: {:?}", + results.code_results.len(), + results.code_results, + ); +} diff --git a/crates/terraphim_mcp_server/src/lib.rs b/crates/terraphim_mcp_server/src/lib.rs index b4a35455..aef05605 100644 --- a/crates/terraphim_mcp_server/src/lib.rs +++ b/crates/terraphim_mcp_server/src/lib.rs @@ -139,6 +139,225 @@ fn no_mmap_budget() -> fff_search::ContentCacheBudget { } } +/// Mirrors fff-search's `PATH_BUF_SIZE` (`simd_path.rs`). +/// +/// Not configurable upstream, so it is reproduced here. If a future fff-search +/// changes it, the guard below becomes conservative rather than unsound. +const PATH_BUF_SIZE: usize = 4096; + +/// The number of bytes `write_absolute_path` consumes before the relative path. +/// +/// Mirrors that function's own arithmetic exactly (`types.rs:343-351`) so the +/// two cannot drift: the base path's encoded bytes, plus one separator unless +/// the base already ends in `MAIN_SEPARATOR`. +/// +/// This counts the root's *raw* bytes, while `terraphim_grep`'s call site hands fff-search +/// `base_path: search_path.to_string_lossy().to_string()`. The two agree only +/// because `reject_hostile_tree` has already refused a root that is not valid +/// UTF-8, which makes `to_string_lossy` a no-op on it. That is the invariant the +/// whole length rule rests on: without it a root with invalid bytes would be +/// stored up to three times longer than measured here, and the arithmetic would +/// understate what the buffer actually holds. +fn base_prefix_len(base: &std::path::Path) -> usize { + let bytes = base.as_os_str().as_encoded_bytes(); + let base_len = bytes.len(); + let sep_len = + usize::from(base_len > 0 && bytes[base_len - 1] != std::path::MAIN_SEPARATOR as u8); + base_len + sep_len +} + +/// Decide one directory entry, returning its length relative to the search root. +/// +/// `base_end_idx` is what `base_prefix_len` returned for the search root: the +/// buffer is shared, so the length rule has to be applied to the total, not to +/// the relative portion alone. +/// +/// Split out from the walk to mirror `terraphim_grep::path_guard`, where both +/// rules are unit-tested. The host filesystem will not create every geometry +/// fff-search mishandles -- macOS caps a single pathname at 1024 bytes and Linux +/// at 4096 -- so the arithmetic has to be exercised through this function. +fn check_entry( + base: &std::path::Path, + dir: &std::path::Path, + name: &std::ffi::OsStr, + base_end_idx: usize, + parent_rel_len: usize, +) -> Result { + if name.to_str().is_none() { + return Err(format!( + "refusing to search {}: entry {:?} under {} has a name that is not \ + valid UTF-8. fff-search stores paths lossily, which inflates each \ + invalid byte threefold, and the shared {PATH_BUF_SIZE}-byte path \ + buffer then truncates inside a replacement character and returns \ + the result through `from_utf8_unchecked` (UF-040 / #296).", + base.display(), + name, + dir.display(), + )); + } + + // Relative length as fff-search stores it: the parent's relative length, a + // separator once there is a parent component, and the name. + let name_len = name.as_encoded_bytes().len(); + let rel_len = if parent_rel_len == 0 { + name_len + } else { + parent_rel_len + 1 + name_len + }; + + // `write_absolute_path` fills one buffer with base, separator and relative, + // so the budget is shared. + // + // Its exact truncation threshold is `base_end_idx + rel_len > PATH_BUF_SIZE`: + // the relative portion is read into `&mut buf[base_end_idx..]`, and + // `read_to_buf` takes `byte_len.min(buf.len())`, so a total of exactly + // `PATH_BUF_SIZE` fits and nothing is cut. This guard uses `>=`, one byte + // stricter, DELIBERATELY -- do not "correct" it to `>`: + // + // - One rule then also covers `write_relative_cstr`, whose own threshold is + // `rel_len >= PATH_BUF_SIZE` because it reserves its buffer's last byte for + // a NUL (`types.rs:366-375`). Since `base_end_idx >= 0`, this condition + // subsumes that one and no second branch is needed. + // - It keeps the guard off the exact dependency boundary, which would + // otherwise have to be re-derived on every fff-search upgrade. + // + // The cost is that an absolute path summing to exactly `PATH_BUF_SIZE` bytes + // is refused although `write_absolute_path` alone would handle it. That is + // intended. + if base_end_idx + rel_len >= PATH_BUF_SIZE { + return Err(format!( + "refusing to search {}: the absolute path to {:?} under {} would be \ + {} bytes ({base_end_idx} for the search root and its separator, plus \ + {rel_len} relative), at or over fff-search's {PATH_BUF_SIZE}-byte path \ + buffer. `write_absolute_path` writes both halves into that one buffer \ + and truncates whatever does not fit, then returns the result through \ + `from_utf8_unchecked` -- which cuts a multi-byte character in half. \ + Searching from a shorter root is usually enough to fix this \ + (UF-040 / #296).", + base.display(), + name, + dir.display(), + base_end_idx + rel_len, + )); + } + + Ok(rel_len) +} + +/// Reject a tree that would defeat fff-search's `from_utf8_unchecked`. +/// +/// fff-search stores every relative path as `to_string_lossy().into_owned()`. +/// On Linux each invalid byte in an entry name becomes a three-byte U+FFFD, so +/// a hostile name inflates threefold. `ChunkedString::read_to_buf` +/// (`simd_path.rs`) then copies `byte_len.min(buf.len())` bytes into a +/// 4096-byte buffer and returns `core::str::from_utf8_unchecked(&buf[..total])` +/// -- so once the inflated path exceeds the buffer the cut lands inside a +/// U+FFFD and the returned `&str` is not valid UTF-8. That is reached from +/// `FileItem::absolute_path`, which both `get_content_for_search` and +/// `collect_files`' git-status lookup call for every file. The `path` argument +/// of these tools is attacker-chosen, so the check has to run before it reaches +/// the library; upstream offers no path-length cap, no strict-UTF-8 mode and no +/// non-lossy storage switch. See UF-040 / #296. +/// +/// Fails, naming the offending entry, when the search root's own path is not +/// valid UTF-8 (it would be stored lossily, inflating it and leaving a name that +/// no longer refers to the directory), when an entry name is not valid UTF-8, +/// when the search root alone fills `PATH_BUF_SIZE` (which makes +/// `write_absolute_path`'s `copy_from_slice` panic rather than truncate), or +/// when the root plus a relative path reaches `PATH_BUF_SIZE`. Base and relative +/// share one buffer (`types.rs:337-357`), so the budget is joint -- the third +/// rule covers ordinary trees of *valid* names reached from a long root, where +/// the truncation cuts a multi-byte character in half with no hostile name +/// involved. EXP-201 did not test that combination, so it is reasoned from +/// `write_absolute_path` and `read_to_buf` rather than observed. +/// +/// Names only -- `DirEntry::file_type` uses the directory entry's type, with no +/// `stat` -- and symbolic links are not descended into, matching the +/// `follow_symlinks: false` the pickers now assert. An unreadable directory is +/// skipped rather than reported: fff-search will skip it too, and failing here +/// would turn a permission quirk into a refused search. +/// +/// Residual risk: a TOCTOU window remains. An entry created between this walk +/// and fff-search's `collect_files` still reaches the inflation. That needs an +/// active concurrent writer inside the searched tree, whereas the confirmed +/// exposure is a static hostile checkout. The walk also does not honour +/// `.gitignore`, so `target/` and `node_modules/` are walked; for a per-call +/// picker that already pays a full `collect_files` scan immediately after, the +/// cost is acceptable. +/// +/// This duplicates `terraphim_grep::path_guard` deliberately: this crate does +/// not depend on `terraphim_grep`, and adding a dependency to share one function +/// would couple two binaries for no benefit. +fn reject_hostile_tree(base: &std::path::Path) -> Result<(), String> { + // fff-search receives `base_path` as a `String`. `terraphim_grep`'s call site + // builds it with `to_string_lossy()`, which replaces each invalid byte with a + // three-byte U+FFFD, so a root with invalid bytes would be stored up to three + // times longer than `base_prefix_len` measures -- defeating the arithmetic + // below, and panicking `copy_from_slice` outright once the inflated root + // alone reaches the buffer. Refusing such a root restores the invariant. + // + // It is the right answer regardless of the arithmetic: the lossily stored + // name no longer names the on-disk entry, so fff-search cannot open anything + // under it. That is the same silent-drop defect the audit recorded for + // hostile entry names, and a named error beats searching and finding nothing. + // + // Reachable through `terraphim-grep --paths` and `with_search_paths`, since + // clap builds `Vec` from `OsString` and invalid bytes survive on + // Linux. The MCP tools are immune -- their `path` arrives as JSON and is + // always valid UTF-8 -- but both copies carry the check so neither depends on + // where it was called from. + // + // The root is never an entry the walk discovers, so the per-entry UTF-8 rule + // in `check_entry` does not cover it. + if base.to_str().is_none() { + return Err(format!( + "refusing to search {:?}: the search root's own path is not valid \ + UTF-8. fff-search stores it lossily, which both inflates each invalid \ + byte threefold past the {PATH_BUF_SIZE}-byte path buffer and leaves a \ + stored name that no longer refers to the directory on disk, so no file \ + under it could be opened (UF-040 / #296).", + base.as_os_str(), + )); + } + + // `write_absolute_path` writes the base path into the same fixed buffer as + // the relative path, so the root's own length is part of the budget -- and a + // root at or over the buffer makes `copy_from_slice` panic outright. + let base_end_idx = base_prefix_len(base); + if base_end_idx >= PATH_BUF_SIZE { + return Err(format!( + "refusing to search {}: the search root alone is {base_end_idx} bytes \ + with its separator, at or over fff-search's {PATH_BUF_SIZE}-byte path \ + buffer, which leaves no room for any relative path. \ + `write_absolute_path` copies the root into that buffer unconditionally, \ + so a root this long panics the library rather than merely truncating \ + (UF-040 / #296).", + base.display(), + )); + } + + let mut stack = vec![(base.to_path_buf(), 0usize)]; + + while let Some((dir, rel_len)) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + + for entry in entries.flatten() { + let name = entry.file_name(); + let entry_rel_len = check_entry(base, &dir, &name, base_end_idx, rel_len)?; + + // `file_type` reports a symbolic link as such, so links are never + // descended into -- no `metadata` call, and no loop to detect. + if entry.file_type().is_ok_and(|t| t.is_dir()) { + stack.push((entry.path(), entry_rel_len)); + } + } + } + + Ok(()) +} + /// The main service type for the Terraphim MCP server #[derive(Clone)] pub struct McpService { @@ -1316,6 +1535,12 @@ impl McpService { limit: Option, ) -> Result { let base_path = path.unwrap_or_else(|| ".".to_string()); + + // Refuse the tree before its base path reaches fff-search: a non-UTF-8 + // or over-long entry name defeats the library's `from_utf8_unchecked` + // on the stored relative path. UF-040 / #296. + reject_hostile_tree(std::path::Path::new(&base_path)) + .map_err(|e| ErrorData::invalid_params(e, None))?; let max_results = limit.unwrap_or(20); // Build index synchronously (no background threads needed for MCP tool use) @@ -1409,6 +1634,12 @@ impl McpService { cursor: Option, ) -> Result { let base_path = path.unwrap_or_else(|| ".".to_string()); + + // Refuse the tree before its base path reaches fff-search: a non-UTF-8 + // or over-long entry name defeats the library's `from_utf8_unchecked` + // on the stored relative path. UF-040 / #296. + reject_hostile_tree(std::path::Path::new(&base_path)) + .map_err(|e| ErrorData::invalid_params(e, None))?; let max_results = limit.unwrap_or(50); let files_only = output_mode.as_deref() == Some("files"); @@ -1522,6 +1753,12 @@ impl McpService { output_mode: Option, ) -> Result { let base_path = path.unwrap_or_else(|| ".".to_string()); + + // Refuse the tree before its base path reaches fff-search: a non-UTF-8 + // or over-long entry name defeats the library's `from_utf8_unchecked` + // on the stored relative path. UF-040 / #296. + reject_hostile_tree(std::path::Path::new(&base_path)) + .map_err(|e| ErrorData::invalid_params(e, None))?; let max_results = limit.unwrap_or(50); let files_only = output_mode.as_deref() == Some("files"); diff --git a/crates/terraphim_mcp_server/tests/test_find_files.rs b/crates/terraphim_mcp_server/tests/test_find_files.rs index 77e2d06a..f904f5a4 100644 --- a/crates/terraphim_mcp_server/tests/test_find_files.rs +++ b/crates/terraphim_mcp_server/tests/test_find_files.rs @@ -234,3 +234,66 @@ async fn grep_files_files_mode_returns_paths() { } } } + +/// `grep_files` must refuse a tree whose entry names are not valid UTF-8, +/// naming the offending entry. +/// +/// fff-search stores relative paths lossily, so each invalid byte becomes a +/// three-byte U+FFFD; `ChunkedString::read_to_buf` then truncates the inflated +/// path into a 4096-byte buffer and returns it through `from_utf8_unchecked`, +/// so the cut lands inside a replacement character. The `path` argument is +/// attacker-chosen, which is what makes this reachable. See UF-040 / #296. +/// +/// Real directories, no mocks. Linux only in practice: APFS rejects non-UTF-8 +/// entry names, so `create_dir` fails on macOS before the guard is reached. +/// `ignore`d rather than `cfg`'d out so the body still compiles and lints here. +#[tokio::test] +#[cfg_attr( + not(target_os = "linux"), + ignore = "APFS refuses non-UTF-8 entry names, so the tree cannot be built here" +)] +#[cfg(unix)] +async fn grep_files_refuses_non_utf8_tree() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let tmp = tempfile::tempdir().expect("tempdir"); + + // 6 x 250 invalid bytes is 1500 raw, inflating to 4500 under lossy storage + // -- comfortably past fff-search's 4096-byte path buffer. + let raw = vec![0xFFu8; 250]; + let segment = OsStr::from_bytes(&raw); + let mut dir = tmp.path().to_path_buf(); + for _ in 0..6 { + dir = dir.join(segment); + std::fs::create_dir(&dir).expect("create nested directory"); + } + std::fs::write(dir.join("payload.rs"), "fn needle() {}\n").expect("write payload"); + + let config_state = minimal_config_state().await; + let service = McpService::new(config_state); + + let err = service + .grep_files( + "needle".to_string(), + Some(tmp.path().to_string_lossy().to_string()), + Some(10), + None, + None, + ) + .await + .expect_err("grep_files over a hostile tree must fail"); + + // Assert on the structured field, not `Display`: this test can never run on + // macOS, so a wrong guess about rmcp's `ErrorData` formatting would only + // surface as a red Linux CI. + let message = err.message.to_string(); + assert!( + message.contains("not valid UTF-8"), + "error must say why the tree was refused, got: {message}" + ); + assert!( + message.contains("\\xff") || message.contains("\\xFF"), + "error must name the offending entry's bytes, got: {message}" + ); +} From 7816b67f4383e41f460cef77410760aaa0c65b14 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 7 Oct 2026 11:27:15 +0100 Subject: [PATCH 3/9] chore(deps): pin fff-search to =0.8.4 and assert the pin in ci_guards (Refs #293, #257) Both dependent crates declared `fff-search = { version = "0.8.4" }`, i.e. `^0.8.4`. Both now declare `=0.8.4`, and a `ci_guards` test asserts the exact pin in each manifest. fff-search 0.10.6 is the first release requiring a git2 that patches RUSTSEC-2026-0183/0184, but it also adds a SECOND, unbudgeted `memmap2::Mmap::map` at `types.rs:762` in `get_content_for_search`, gated only by a 256 KiB threshold on Linux and reached exactly when a zero-file cache budget makes `get_cached_content` return `None`. Upgrading would therefore reintroduce UF-039 / #293 for every file at or above that size while leaving the mitigation visibly in place -- the worst possible failure mode for a fix whose whole point is that nothing maps any more. Staying on 0.8.4 is the audit's decision (Delta 5, scored 19 against 12/10/8) and is safe because both advisories are API-specific and neither vulnerable API is reachable: fff-search 0.8.4 references only `git2::Repository`, `git2::Error` and the `git2::Status` flags, with no `Remote`, `Blame` or `blame_buffer` anywhere, and no workspace crate depends on git2 directly. The urgency, stated accurately: `^0.8.4` already confined cargo to 0.8.x, and crates.io has no stable 0.8.x above 0.8.4 -- only `0.8.5-nightly.*` pre-releases, which cargo will not select. A routine `cargo update` therefore cannot reach 0.10.6 today. The pin guards against a deliberate minor bump, and it is what stops the reachability argument above lapsing silently. The earlier claim that a routine update would reopen the finding was retracted during the audit and is not repeated here. `Cargo.lock` is unchanged: the exact requirement matches what was already resolved. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjGks4DGYdDw3QLgu32S96 --- crates/terraphim_agent/tests/ci_guards.rs | 33 +++++++++++++++++++++++ crates/terraphim_grep/Cargo.toml | 23 +++++++++++++++- crates/terraphim_mcp_server/Cargo.toml | 23 +++++++++++++++- 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/crates/terraphim_agent/tests/ci_guards.rs b/crates/terraphim_agent/tests/ci_guards.rs index 407d826b..c09aa463 100644 --- a/crates/terraphim_agent/tests/ci_guards.rs +++ b/crates/terraphim_agent/tests/ci_guards.rs @@ -786,3 +786,36 @@ fn every_file_picker_refuses_the_mmap_cache() { } } } + +/// Both crates that depend on fff-search must pin it exactly. +/// +/// fff-search 0.10.6 adds a second, unbudgeted `memmap2::Mmap::map` in +/// `get_content_for_search`, reached exactly when the zero-file cache budget +/// from #293 makes `get_cached_content` return `None` -- so a minor bump would +/// reintroduce UF-039 for every file at or above its 256 KiB threshold while +/// leaving the mitigation visibly in place. `^0.8.4` already confines cargo to +/// 0.8.x and no stable 0.8.x above 0.8.4 exists, so this pin is not urgent; it +/// guards against a deliberate bump and keeps the decision from lapsing +/// silently. See Delta 5 of the audit; Refs #293, #257. +#[test] +fn fff_search_is_pinned_exactly() { + let root = workspace_root(); + for crate_name in ["terraphim_grep", "terraphim_mcp_server"] { + let manifest = root.join("crates").join(crate_name).join("Cargo.toml"); + let text = std::fs::read_to_string(&manifest) + .unwrap_or_else(|e| panic!("read {}: {e}", manifest.display())); + + let decl = text + .lines() + .find(|l| l.trim_start().starts_with("fff-search = ")) + .unwrap_or_else(|| panic!("{} declares no fff-search dependency", manifest.display())); + + assert!( + decl.contains(r#"version = "=0.8.4""#), + "{} must pin fff-search exactly (`version = \"=0.8.4\"`); a caret \ + requirement lets a minor bump reintroduce UF-039 (Refs #293, #257). \ + Found: {decl}", + manifest.display(), + ); + } +} diff --git a/crates/terraphim_grep/Cargo.toml b/crates/terraphim_grep/Cargo.toml index 5d155186..1a01eaa7 100644 --- a/crates/terraphim_grep/Cargo.toml +++ b/crates/terraphim_grep/Cargo.toml @@ -36,7 +36,28 @@ terraphim_automata = { version = "1.21.0", registry = "terraphim" } terraphim_service = { version = "1.21.1", optional = true, registry = "terraphim" } terraphim_config = { version = "1.15.0" } -fff-search = { version = "0.8.4", optional = true } +# Pinned exactly, not `^0.8.4` (Refs #293, #257). +# +# fff-search 0.10.6 is the first release requiring a git2 that patches +# RUSTSEC-2026-0183/0184, but it also adds a SECOND, unbudgeted +# `memmap2::Mmap::map` at `types.rs:762` in `get_content_for_search`, gated only +# by a 256 KiB threshold on Linux and reached exactly when a zero-file cache +# budget makes `get_cached_content` return `None`. Upgrading would therefore +# reintroduce UF-039 for every file at or above that size while leaving the +# mitigation visibly in place. +# +# Staying on 0.8.4 is safe because both advisories are API-specific and neither +# vulnerable API is reachable: fff-search 0.8.4 references only +# `git2::Repository`, `git2::Error` and the `git2::Status` flags, with no +# `Remote`, `Blame` or `blame_buffer` anywhere, and no workspace crate depends +# on git2 directly. +# +# Urgency, stated accurately: `^0.8.4` already confines cargo to 0.8.x and +# crates.io has no stable 0.8.x above 0.8.4 (only `0.8.5-nightly.*` +# pre-releases, which cargo will not select), so a routine `cargo update` cannot +# reach 0.10.6 today. The pin guards against a deliberate minor bump, and it is +# what stops the reachability argument above lapsing silently. +fff-search = { version = "=0.8.4", optional = true } async-trait.workspace = true reqwest = { workspace = true, features = ["json", "rustls-tls"], optional = true } diff --git a/crates/terraphim_mcp_server/Cargo.toml b/crates/terraphim_mcp_server/Cargo.toml index c9a03e72..c4acd83c 100644 --- a/crates/terraphim_mcp_server/Cargo.toml +++ b/crates/terraphim_mcp_server/Cargo.toml @@ -22,7 +22,28 @@ clap = { version = "4.5", features = ["derive"] } rmcp = { version = "0.9.0", features = ["server", "transport-sse-server", "transport-io"] } serde_json = { workspace = true } -fff-search = { version = "0.8.4" } +# Pinned exactly, not `^0.8.4` (Refs #293, #257). +# +# fff-search 0.10.6 is the first release requiring a git2 that patches +# RUSTSEC-2026-0183/0184, but it also adds a SECOND, unbudgeted +# `memmap2::Mmap::map` at `types.rs:762` in `get_content_for_search`, gated only +# by a 256 KiB threshold on Linux and reached exactly when a zero-file cache +# budget makes `get_cached_content` return `None`. Upgrading would therefore +# reintroduce UF-039 for every file at or above that size while leaving the +# mitigation visibly in place. +# +# Staying on 0.8.4 is safe because both advisories are API-specific and neither +# vulnerable API is reachable: fff-search 0.8.4 references only +# `git2::Repository`, `git2::Error` and the `git2::Status` flags, with no +# `Remote`, `Blame` or `blame_buffer` anywhere, and no workspace crate depends +# on git2 directly. +# +# Urgency, stated accurately: `^0.8.4` already confines cargo to 0.8.x and +# crates.io has no stable 0.8.x above 0.8.4 (only `0.8.5-nightly.*` +# pre-releases, which cargo will not select), so a routine `cargo update` cannot +# reach 0.10.6 today. The pin guards against a deliberate minor bump, and it is +# what stops the reachability argument above lapsing silently. +fff-search = { version = "=0.8.4" } terraphim_automata = { version = "1.21.0", registry = "terraphim", features = ["tokio-runtime"] } terraphim_config = { version = "1.20.2" } terraphim_file_search = { version = "1.20.3" } From f83539bc24192e265bc91fca7bae4432a905443e Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Sun, 11 Oct 2026 18:18:46 +0100 Subject: [PATCH 4/9] fix: retrieve KG synonyms and honor requested grep synthesis --- crates/terraphim_grep/src/hybrid_searcher.rs | 64 +++++++++- crates/terraphim_grep/src/lib.rs | 11 ++ .../terraphim_grep/src/sufficiency_judge.rs | 54 ++++++++- crates/terraphim_grep/tests/answer_request.rs | 54 +++++++++ .../terraphim_grep/tests/synonym_retrieval.rs | 112 ++++++++++++++++++ 5 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 crates/terraphim_grep/tests/answer_request.rs create mode 100644 crates/terraphim_grep/tests/synonym_retrieval.rs diff --git a/crates/terraphim_grep/src/hybrid_searcher.rs b/crates/terraphim_grep/src/hybrid_searcher.rs index e8ccf376..190ae7ab 100644 --- a/crates/terraphim_grep/src/hybrid_searcher.rs +++ b/crates/terraphim_grep/src/hybrid_searcher.rs @@ -126,7 +126,7 @@ pub fn score_kg_boost(chunk: &RetrievedChunk, concepts: &[KgConcept], weight: f6 (boost * weight).min(weight * concepts.len() as f64) } -fn automata_concept_matches(text: &str, concept: &str) -> bool { +pub(crate) fn automata_concept_matches(text: &str, concept: &str) -> bool { let role = terraphim_types::RoleName::new("terraphim-grep-kg-boost"); let thesaurus = terraphim_automata::thesaurus_from_terms(&role, std::iter::once(concept)); match terraphim_automata::find_matches(text, &thesaurus, true) { @@ -196,6 +196,35 @@ fn thesaurus_query_concepts( } } +/// Expand an exact KG term into its existing normalized concept's aliases. +/// Compound/free-text queries retain their original constraints. +fn retrieval_queries(query: &str, thesaurus: &terraphim_types::Thesaurus) -> Vec { + let mut queries = vec![query.to_owned()]; + let Ok(matches) = terraphim_automata::find_matches(query.trim(), thesaurus, false) else { + return queries; + }; + let exact = matches + .into_iter() + .find(|m| m.term.eq_ignore_ascii_case(query.trim())); + if let Some(matched) = exact { + // Resolve exact membership before the presentation limit, and always include + // the canonical term even when an alias group exceeds that limit. + let canonical = matched.normalized_term.value.to_string(); + if !queries.iter().any(|q| q.eq_ignore_ascii_case(&canonical)) { + queries.push(canonical); + } + for concept in thesaurus_query_concepts(query, thesaurus, 32) { + if !queries + .iter() + .any(|q| q.eq_ignore_ascii_case(&concept.name)) + { + queries.push(concept.name); + } + } + } + queries +} + /// Apply KG boost to a batch of chunks and sort by boosted score (descending). /// Mutates `relevance_score` in place so downstream consumers can see the boost reflected /// in the JSON output -- otherwise the ordering would be inexplicable. @@ -318,11 +347,27 @@ impl HybridSearcher { } else { max_results.saturating_mul(5).max(max_results).min(1000) }; + let queries = retrieval_queries(&query_owned, &thesaurus); let mut all_results = Vec::new(); for path in search_paths.iter() { - let mut results = - Self::search_code(&query_owned, candidate_limit, path.clone()).await?; - all_results.append(&mut results); + // Deduplicate one actual line matched by several aliases. Keep + // path roots distinct: relative filenames can coincide in two roots. + let mut seen = std::collections::HashSet::new(); + for query in &queries { + let results = + Self::search_code(query, candidate_limit, path.clone()).await?; + for result in results { + let key = ( + result.source.clone(), + result.line_start, + result.line_end, + result.content.clone(), + ); + if seen.insert(key) { + all_results.push(result); + } + } + } } // Boost KG matches above generic matches; the boosted score is what the // JSON output reports so downstream tools see why a chunk ranked where @@ -680,6 +725,17 @@ mod tests { assert_eq!(names, vec!["auth", "authentication", "authorisation"]); } + #[test] + fn retrieval_preserves_compound_and_unknown_queries() { + let thesaurus = test_thesaurus(&["auth", "authorisation"]); + assert_eq!( + retrieval_queries("auth AND deployment", &thesaurus), + vec!["auth AND deployment"] + ); + assert_eq!(retrieval_queries("author", &thesaurus), vec!["author"]); + assert_eq!(retrieval_queries("unknown", &thesaurus), vec!["unknown"]); + } + #[test] fn kg_boost_promotes_matching_chunks_to_top() { // Two chunks with identical base scores. Only one mentions the KG concept in its diff --git a/crates/terraphim_grep/src/lib.rs b/crates/terraphim_grep/src/lib.rs index ff2c44dd..d2ff78dc 100644 --- a/crates/terraphim_grep/src/lib.rs +++ b/crates/terraphim_grep/src/lib.rs @@ -199,6 +199,17 @@ impl TerraphimGrep { let thresholds = self.sufficiency_judge.thresholds(); match sufficiency { + sufficiency_judge::Sufficiency::Sufficient(chunks) if Self::rlm_requested(&options) => { + self.search_with_rlm_fallback( + query, + options, + chunks, + hybrid_results, + start, + metrics, + ) + .await + } sufficiency_judge::Sufficiency::Sufficient(chunks) => Ok(Self::search_only_result( chunks, hybrid_results, diff --git a/crates/terraphim_grep/src/sufficiency_judge.rs b/crates/terraphim_grep/src/sufficiency_judge.rs index d12b198e..32a64ae8 100644 --- a/crates/terraphim_grep/src/sufficiency_judge.rs +++ b/crates/terraphim_grep/src/sufficiency_judge.rs @@ -70,7 +70,7 @@ impl SufficiencyJudge { let metrics = SufficiencyMetrics { chunk_count: chunks.len(), kg_hits: results.kg_concepts.len(), - coverage: self.calculate_coverage(query, &chunks), + coverage: self.calculate_semantic_coverage(query, &chunks, &results.kg_concepts), kg_confidence: self.calculate_kg_confidence(&results.kg_concepts), diversity: self.calculate_diversity(&chunks), }; @@ -98,6 +98,29 @@ impl SufficiencyJudge { (decision, metrics) } + fn calculate_semantic_coverage( + &self, + query: &str, + chunks: &[RetrievedChunk], + concepts: &[super::hybrid_searcher::KgConcept], + ) -> f64 { + // Only an exact KG term can substitute its aliases. Compound queries + // still require their literal terms; KG hits without retrieved evidence + // cannot establish coverage. + if concepts + .iter() + .any(|c| c.name.eq_ignore_ascii_case(query.trim())) + && chunks.iter().any(|chunk| { + concepts.iter().any(|concept| { + super::hybrid_searcher::automata_concept_matches(&chunk.content, &concept.name) + }) + }) + { + return 1.0; + } + self.calculate_coverage(query, chunks) + } + fn calculate_coverage(&self, query: &str, chunks: &[RetrievedChunk]) -> f64 { if chunks.is_empty() { return 0.0; @@ -222,6 +245,35 @@ mod tests { assert!(coverage2 < 0.5); } + #[test] + fn semantic_coverage_requires_exact_query_and_real_boundary_match() { + let judge = SufficiencyJudge::default(); + let concepts = ["judgment", "decision"].map(|name| KgConcept { + id: 1, + name: name.into(), + display_value: None, + score: 1.0, + }); + let chunks = vec![make_chunk("Decision: public preview", "brief.md", "code")]; + assert_eq!( + judge.calculate_semantic_coverage("judgment", &chunks, &concepts), + 1.0 + ); + assert_eq!( + judge.calculate_semantic_coverage("judgment missing", &chunks, &concepts), + 0.0 + ); + let unrelated = vec![make_chunk("indecision", "decision.md", "code")]; + assert_eq!( + judge.calculate_semantic_coverage("judgment", &unrelated, &concepts), + 0.0 + ); + assert_eq!( + judge.calculate_semantic_coverage("judgment", &[], &concepts), + 0.0 + ); + } + #[test] fn test_diversity_calculation() { let judge = SufficiencyJudge::default(); diff --git a/crates/terraphim_grep/tests/answer_request.rs b/crates/terraphim_grep/tests/answer_request.rs new file mode 100644 index 00000000..251106d9 --- /dev/null +++ b/crates/terraphim_grep/tests/answer_request.rs @@ -0,0 +1,54 @@ +//! Explicit synthesis requests must not be discarded by the sufficiency heuristic. +#![cfg(all(feature = "llm", feature = "code-search"))] + +use std::sync::Arc; +use terraphim_grep::{ + GrepOptions, HeuristicThresholds, HybridSearcher, SufficiencyJudge, TerraphimGrep, +}; +use terraphim_types::Thesaurus; + +#[tokio::test] +async fn sufficient_results_honor_answer_request_without_inventing_an_answer() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("brief.md"), "Decision: public preview.\n").unwrap(); + let searcher = HybridSearcher::new("public-demo".into(), Thesaurus::new("public-demo".into())) + .unwrap() + .with_search_path(root.path().to_path_buf()); + let judge = SufficiencyJudge::new(HeuristicThresholds { + min_coverage: 0.0, + min_kg_confidence: 0.0, + min_diversity: 0, + min_results: 1, + }); + // Real retrieval/judge, deliberately no provider: requested synthesis must explain + // the unavailable provider rather than claim search answered the request. + let grep = TerraphimGrep::new(Arc::new(searcher), Arc::new(judge)); + let plain = grep + .search("Decision", GrepOptions::default()) + .await + .unwrap(); + assert!( + plain + .sufficiency_explanation + .contains("Results sufficient on their own") + ); + let answer = grep + .search( + "Decision", + GrepOptions { + include_answer: true, + ..GrepOptions::default() + }, + ) + .await + .unwrap(); + assert_eq!(plain.chunks.len(), answer.chunks.len()); + assert!(answer.answer.is_none()); + assert!( + answer + .sufficiency_explanation + .contains("no LLM client is configured"), + "{}", + answer.sufficiency_explanation + ); +} diff --git a/crates/terraphim_grep/tests/synonym_retrieval.rs b/crates/terraphim_grep/tests/synonym_retrieval.rs new file mode 100644 index 00000000..4a078475 --- /dev/null +++ b/crates/terraphim_grep/tests/synonym_retrieval.rs @@ -0,0 +1,112 @@ +//! Real CLI/file regression: concept lookup must retrieve synonym-only documents. +#![cfg(feature = "code-search")] + +use std::process::Command; + +#[test] +fn alias_retrieves_canonical_file_without_duplicate_lines_or_log_matches() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write( + corpus.join("brief.md"), + "Decision: launch the public workspace.\nDecision and judgment share one concept.\n", + ) + .unwrap(); + let graph = root.path().join("thesaurus.json"); + std::fs::write( + &graph, + r#"{"name":"public-demo","data":{"decision":{"id":1,"nterm":"decision"},"judgment":{"id":1,"nterm":"decision"},"choice":{"id":1,"nterm":"decision"}}}"#, + ) + .unwrap(); + // This evidence-looking file is deliberately outside the searched corpus. + std::fs::write(root.path().join("receipt.json"), "judgment").unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["judgment", "--search-only", "--json", "--thesaurus"]) + .arg(&graph) + .arg("--paths") + .arg(&corpus) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let chunks = result["chunks"].as_array().unwrap(); + assert_eq!(chunks.len(), 2, "actual output: {result}"); + assert!(chunks.iter().all(|c| c["source"] == "brief.md")); + assert!(chunks.iter().any(|c| c["line_start"] == 1)); + assert!(chunks.iter().any(|c| c["line_start"] == 2)); + assert!(result["answer"].is_null()); +} + +#[test] +fn late_sorting_alias_in_large_concept_still_retrieves_canonical_text() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write(corpus.join("brief.md"), "Decision: public preview.\n").unwrap(); + let mut data = serde_json::Map::new(); + for name in (0..40) + .map(|i| format!("alias{i:02}")) + .chain(["decision".into(), "zzjudgment".into()]) + { + data.insert(name, serde_json::json!({"id":1,"nterm":"decision"})); + } + let graph = root.path().join("thesaurus.json"); + std::fs::write( + &graph, + serde_json::to_vec(&serde_json::json!({"name":"large", "data":data})).unwrap(), + ) + .unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["zzjudgment", "--search-only", "--json", "--thesaurus"]) + .arg(&graph) + .arg("--paths") + .arg(&corpus) + .output() + .unwrap(); + assert!(output.status.success()); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["chunks"].as_array().unwrap().len(), 1, "{result}"); + assert_eq!(result["chunks"][0]["source"], "brief.md"); +} + +#[test] +fn synonym_evidence_counts_towards_synthesis_coverage() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + for index in 0..3 { + std::fs::write( + corpus.join(format!("brief{index}.md")), + "Decision: public preview.\n", + ) + .unwrap(); + } + let graph = root.path().join("thesaurus.json"); + std::fs::write(&graph, r#"{"name":"public-demo","data":{"decision":{"id":1,"nterm":"decision"},"judgment":{"id":1,"nterm":"decision"}}}"#).unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["judgment", "--answer", "--json", "--thesaurus"]) + .arg(graph) + .arg("--paths") + .arg(corpus) + .env_remove("OPENROUTER_API_KEY") + .env_remove("OLLAMA_BASE_URL") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["chunks"].as_array().unwrap().len(), 3); + assert_ne!(result["sufficiency"], "RlmInsufficient", "{result}"); + assert!( + result["answer"].is_null(), + "no configured provider must not invent output" + ); +} From 815329712095b594ca00e7ecb9859b097d282b07 Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Sun, 11 Oct 2026 22:43:26 +0100 Subject: [PATCH 5/9] fix: preserve decision status in synthesized answers --- crates/terraphim_grep/src/rlm_context.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/terraphim_grep/src/rlm_context.rs b/crates/terraphim_grep/src/rlm_context.rs index 401ffa0c..8ed333f7 100644 --- a/crates/terraphim_grep/src/rlm_context.rs +++ b/crates/terraphim_grep/src/rlm_context.rs @@ -46,7 +46,10 @@ impl RlmContext { } pub fn build_prompt(&self) -> String { - let mut prompt = format!("Query: {}\n\n", self.query); + let mut prompt = String::from( + "Preserve source modality, tense and action status. Describe Decision, Plan and Policy items as decisions, plans and policies; never imply implementation or completion unless the retrieved source explicitly confirms it. If completion status is absent, say it is not established.\n\n", + ); + prompt.push_str(&format!("Query: {}\n\n", self.query)); if !self.retrieved_chunks.is_empty() { prompt.push_str("## Retrieved Context\n\n"); @@ -168,6 +171,23 @@ mod tests { assert!(prompt.contains("retry")); } + #[test] + fn synthesis_preserves_decisions_without_claiming_completed_actions() { + let source = "Risk: Rain may delay planting. Decision: Reserve a covered area."; + let ctx = RlmContext::new("concern".into()).with_chunks(vec![make_chunk( + source, + "project-brief.md", + "code", + )]); + let prompt = ctx.build_prompt(); + assert!(prompt.contains(source)); + assert!(prompt.contains("Preserve source modality, tense and action status")); + assert!(prompt.contains("never imply implementation or completion")); + assert!( + prompt.find("Preserve source").unwrap() < prompt.find("## Retrieved Context").unwrap() + ); + } + #[test] fn test_truncate() { let chunks = vec![ From 7a3a0399406211a81413389769c58175dd2ca235 Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Sun, 11 Oct 2026 23:27:34 +0100 Subject: [PATCH 6/9] fix(grep): preserve full literal synonym retrieval and coverage --- crates/terraphim_grep/README.md | 57 +++++++- crates/terraphim_grep/src/hybrid_searcher.rs | 108 +++++++------- crates/terraphim_grep/src/lib.rs | 16 +- .../terraphim_grep/src/sufficiency_judge.rs | 27 +++- .../terraphim_grep/tests/synonym_retrieval.rs | 138 ++++++++++++++++++ 5 files changed, 283 insertions(+), 63 deletions(-) diff --git a/crates/terraphim_grep/README.md b/crates/terraphim_grep/README.md index 9388486f..5d4614b3 100644 --- a/crates/terraphim_grep/README.md +++ b/crates/terraphim_grep/README.md @@ -4,11 +4,12 @@ Intelligent hybrid grep with RLM (Retrieval-Language Model) fallback and knowled ## Overview -`terraphim_grep` is a search crate that combines local code search, knowledge-graph (KG) concept boosting, and optional LLM synthesis into a single pipeline. It runs code retrieval and KG concept lookup in parallel, applies KG-aware boosting so chunks matching your thesaurus rank above generic matches, and falls back to an LLM only when local sufficiency is below threshold. +`terraphim_grep` is a search crate that combines local code search, knowledge-graph (KG) concept boosting, and optional LLM synthesis into a single pipeline. It retrieves local evidence using the configured thesaurus, applies KG-aware boosting, and optionally synthesizes an answer when explicitly requested and enough evidence is available. ## Features -- **Hybrid Search**: Parallel execution of code search and KG concept lookup +- **Hybrid Search**: Local code search combined with KG concept lookup +- **Synonym Retrieval**: An exact KG term searches every alias of the same normalized concept - **KG-Aware Boosting**: Chunks matching thesaurus concepts rank higher than generic matches - **Sufficiency Judging**: Heuristic-based decision on whether results are sufficient or need LLM synthesis - **RLM Fallback**: Graceful degradation to search-only mode when no LLM is configured @@ -102,6 +103,50 @@ terraphim-grep "struct Config" --paths src/ crates/ > (terraphim/terraphim-clients#81). Use `--search-only` (alias `--no-rlm`) to also skip > building the LLM client. +### Use an editor-exported synonym graph + +In Terraphim Editor, add `Concern` as an alternative to `Risk`, then open +**Knowledge graph** and download the graph JSON and the document text. Store the +text under a dedicated corpus directory and keep the graph and run receipts +outside that directory: + +```text +project/ + knowledge-graph.json + corpus/ + project-brief.md +``` + +The editor's `synonyms:: Risk, Concern` group uses Terraphim's existing normalized +term types. With that exported graph, an exact query for `concern` retrieves text +written with `risk`, even when `concern` is absent from the document: + +```bash +# Original passages and locations; no synthesis requested +terraphim-grep concern --thesaurus knowledge-graph.json --paths corpus/ --json + +# Same retrieval, with a requested source-grounded answer +terraphim-grep concern --thesaurus knowledge-graph.json --paths corpus/ --json --answer +``` + +Exact concept queries use all aliases, including aliases beyond the displayed +concept limit. `-n` bounds returned passages and displayed concepts; it does not +truncate the synonym group used for retrieval or evidence coverage. Canonical +terms and aliases are searched as literal text, so an alternative containing +`/team/` or `type:rust` cannot become a filename filter. Multiple aliases matching +one source line produce one passage. Queries that are not exactly a KG term keep +the normal query syntax: `concern *.md`, for example, searches literal `concern` +with the Markdown filter and does not expand synonyms. + +`--answer` requests synthesis even when the heuristic considers search results +sufficient on their own. The default evidence threshold remains three passages; +a smaller result limit or missing evidence can still make the result +`RlmInsufficient`. Without a configured provider, the result retains the actual +passages, explains that no LLM is available, and leaves `answer` null. Synthesized +answers are instructed to preserve source modality and distinguish a decision +or plan from an action confirmed as completed. Check the returned citations +against the source; instructions alone are not a guarantee of grounding. + ## Architecture ``` @@ -110,7 +155,7 @@ Query ▼ ┌──────────────────┐ │ HybridSearcher │──→ Code search (fff-search) ──┐ -│ (parallel) │──→ KG concept lookup ────────┤ +│ │──→ KG concept lookup ────────┤ └──────────────────┘ │ │ ▼ ▼ ┌──────────────────┐ @@ -119,16 +164,16 @@ Query │ (heuristics) │ └──────────────────┘ └──────────────────┘ │ - ├── Sufficient ──→ Return chunks (SearchOnly) + ├── Sufficient ──→ --answer/--force-rlm ? RLM fallback : chunks (SearchOnly) ├── NeedsSynthesis ──→ --answer/--force-rlm ? RLM fallback : chunks (SearchOnly) ├── NeedsExpansion ──→ --answer/--force-rlm ? RLM fallback + extra chunks : chunks (SearchOnly) - └── Insufficient ──→ Return empty (RlmInsufficient) + └── Insufficient ──→ Return retrieved chunks (RlmInsufficient) ``` ## Key Types - **`TerraphimGrep`** — Main entry point combining search, sufficiency judging, and optional LLM -- **`HybridSearcher`** — Parallel code search and KG concept lookup with boosting +- **`HybridSearcher`** — Synonym-aware code search and KG concept lookup with boosting - **`SufficiencyJudge`** — Heuristic thresholds for deciding if results need LLM synthesis - **`GrepOptions`** — Query configuration (haystack, context lines, max results, force RLM) - **`GrepResult`** — Search output with chunks, optional answer, concepts, and stats diff --git a/crates/terraphim_grep/src/hybrid_searcher.rs b/crates/terraphim_grep/src/hybrid_searcher.rs index 190ae7ab..9987e564 100644 --- a/crates/terraphim_grep/src/hybrid_searcher.rs +++ b/crates/terraphim_grep/src/hybrid_searcher.rs @@ -196,33 +196,29 @@ fn thesaurus_query_concepts( } } -/// Expand an exact KG term into its existing normalized concept's aliases. -/// Compound/free-text queries retain their original constraints. -fn retrieval_queries(query: &str, thesaurus: &terraphim_types::Thesaurus) -> Vec { - let mut queries = vec![query.to_owned()]; +/// Resolve every alias of an exact KG term independently of display limits. +/// Compound/free-text queries return no aliases and retain query-parser semantics. +fn exact_query_aliases(query: &str, thesaurus: &terraphim_types::Thesaurus) -> Vec { let Ok(matches) = terraphim_automata::find_matches(query.trim(), thesaurus, false) else { - return queries; + return Vec::new(); }; - let exact = matches + let Some(matched) = matches .into_iter() - .find(|m| m.term.eq_ignore_ascii_case(query.trim())); - if let Some(matched) = exact { - // Resolve exact membership before the presentation limit, and always include - // the canonical term even when an alias group exceeds that limit. - let canonical = matched.normalized_term.value.to_string(); - if !queries.iter().any(|q| q.eq_ignore_ascii_case(&canonical)) { - queries.push(canonical); - } - for concept in thesaurus_query_concepts(query, thesaurus, 32) { - if !queries - .iter() - .any(|q| q.eq_ignore_ascii_case(&concept.name)) - { - queries.push(concept.name); - } - } - } - queries + .find(|m| m.term.eq_ignore_ascii_case(query.trim())) + else { + return Vec::new(); + }; + let canonical = matched.normalized_term.value; + let mut aliases: Vec = thesaurus + .into_iter() + .filter(|(_, term)| term.value == canonical) + .map(|(alias, _)| alias.to_string()) + .chain(std::iter::once(canonical.to_string())) + .filter(|alias| !alias.trim().is_empty()) + .collect(); + aliases.sort(); + aliases.dedup(); + aliases } /// Apply KG boost to a batch of chunks and sort by boosted score (descending). @@ -325,6 +321,11 @@ impl HybridSearcher { self } + /// Full exact membership used by retrieval and sufficiency, not presentation. + pub(crate) fn exact_query_aliases(&self, query: &str) -> Vec { + exact_query_aliases(query, &self.thesaurus) + } + pub async fn search( &self, query: &str, @@ -347,27 +348,13 @@ impl HybridSearcher { } else { max_results.saturating_mul(5).max(max_results).min(1000) }; - let queries = retrieval_queries(&query_owned, &thesaurus); + let aliases = exact_query_aliases(&query_owned, &thesaurus); let mut all_results = Vec::new(); for path in search_paths.iter() { - // Deduplicate one actual line matched by several aliases. Keep - // path roots distinct: relative filenames can coincide in two roots. - let mut seen = std::collections::HashSet::new(); - for query in &queries { - let results = - Self::search_code(query, candidate_limit, path.clone()).await?; - for result in results { - let key = ( - result.source.clone(), - result.line_start, - result.line_end, - result.content.clone(), - ); - if seen.insert(key) { - all_results.push(result); - } - } - } + let results = + Self::search_code(&query_owned, &aliases, candidate_limit, path.clone()) + .await?; + all_results.extend(results); } // Boost KG matches above generic matches; the boosted score is what the // JSON output reports so downstream tools see why a chunk ranked where @@ -430,6 +417,7 @@ impl HybridSearcher { async fn search_code( query: &str, + aliases: &[String], limit: usize, search_path: PathBuf, ) -> Result, String> { @@ -468,7 +456,6 @@ impl HybridSearcher { return Ok(vec![]); } - let fff_query = parse_grep_query(query); let options = GrepSearchOptions { max_file_size: MAX_GREP_FILE_SIZE, max_matches_per_file: 200, @@ -483,8 +470,19 @@ impl HybridSearcher { ..GrepSearchOptions::default() }; - let result = picker.grep(&fff_query, &options); + // Exact KG aliases are data, so bypass the query parser entirely. + // One Aho-Corasick batch scans each file once, including aliases that + // sort after the displayed concepts. Free-text queries retain filters. + let fff_query; + let result = if aliases.is_empty() { + fff_query = parse_grep_query(query); + picker.grep(&fff_query, &options) + } else { + let patterns: Vec<&str> = aliases.iter().map(String::as_str).collect(); + picker.multi_grep(&patterns, &[], &options) + }; + let mut seen = std::collections::HashSet::new(); let chunks = result .matches .iter() @@ -500,6 +498,16 @@ impl HybridSearcher { haystack: "code", }) }) + // Keep one actual line even if several aliases match it. This + // set is local to a root, so equal filenames across roots survive. + .filter(|chunk| { + seen.insert(( + chunk.source.clone(), + chunk.line_start, + chunk.line_end, + chunk.content.clone(), + )) + }) .collect(); Ok(chunks) @@ -507,7 +515,7 @@ impl HybridSearcher { #[cfg(not(feature = "code-search"))] { - let _ = (query, limit, search_path); + let _ = (query, aliases, limit, search_path); // `code-search` is a default feature, so reaching here means the binary was // built with `--no-default-features` (or a custom set omitting it). Returning // an empty Vec silently makes every query look like "0 results" with no @@ -729,11 +737,11 @@ mod tests { fn retrieval_preserves_compound_and_unknown_queries() { let thesaurus = test_thesaurus(&["auth", "authorisation"]); assert_eq!( - retrieval_queries("auth AND deployment", &thesaurus), - vec!["auth AND deployment"] + exact_query_aliases("auth AND deployment", &thesaurus), + Vec::::new() ); - assert_eq!(retrieval_queries("author", &thesaurus), vec!["author"]); - assert_eq!(retrieval_queries("unknown", &thesaurus), vec!["unknown"]); + assert!(exact_query_aliases("author", &thesaurus).is_empty()); + assert!(exact_query_aliases("unknown", &thesaurus).is_empty()); } #[test] diff --git a/crates/terraphim_grep/src/lib.rs b/crates/terraphim_grep/src/lib.rs index d2ff78dc..f647b870 100644 --- a/crates/terraphim_grep/src/lib.rs +++ b/crates/terraphim_grep/src/lib.rs @@ -193,9 +193,11 @@ impl TerraphimGrep { let search_latency_ms = start.elapsed().as_millis() as u64; - let (sufficiency, metrics) = self - .sufficiency_judge - .judge_with_metrics(&hybrid_results, query); + let (sufficiency, metrics) = self.sufficiency_judge.judge_with_aliases( + &hybrid_results, + query, + Some(&self.hybrid_searcher.exact_query_aliases(query)), + ); let thresholds = self.sufficiency_judge.thresholds(); match sufficiency { @@ -521,9 +523,11 @@ impl TerraphimGrep { .await .map_err(TerraphimGrepError::SearchFailed)?; - let (_sufficiency, metrics) = self - .sufficiency_judge - .judge_with_metrics(&hybrid_results, query); + let (_sufficiency, metrics) = self.sufficiency_judge.judge_with_aliases( + &hybrid_results, + query, + Some(&self.hybrid_searcher.exact_query_aliases(query)), + ); self.search_with_rlm_fallback( query, diff --git a/crates/terraphim_grep/src/sufficiency_judge.rs b/crates/terraphim_grep/src/sufficiency_judge.rs index 32a64ae8..78db61ec 100644 --- a/crates/terraphim_grep/src/sufficiency_judge.rs +++ b/crates/terraphim_grep/src/sufficiency_judge.rs @@ -64,13 +64,38 @@ impl SufficiencyJudge { &self, results: &HybridResults, query: &str, + ) -> (Sufficiency, SufficiencyMetrics) { + self.judge_with_aliases(results, query, None) + } + + /// Use complete exact membership when the caller owns the thesaurus. `None` + /// preserves the standalone judge API; `Some` separates evidence from the + /// displayed concepts, including an empty set for non-exact queries. + pub(crate) fn judge_with_aliases( + &self, + results: &HybridResults, + query: &str, + exact_aliases: Option<&[String]>, ) -> (Sufficiency, SufficiencyMetrics) { let chunks = results.to_chunks(); + let coverage = match exact_aliases { + Some(aliases) + if chunks.iter().any(|chunk| { + aliases.iter().any(|alias| { + super::hybrid_searcher::automata_concept_matches(&chunk.content, alias) + }) + }) => + { + 1.0 + } + Some(_) => self.calculate_coverage(query, &chunks), + None => self.calculate_semantic_coverage(query, &chunks, &results.kg_concepts), + }; let metrics = SufficiencyMetrics { chunk_count: chunks.len(), kg_hits: results.kg_concepts.len(), - coverage: self.calculate_semantic_coverage(query, &chunks, &results.kg_concepts), + coverage, kg_confidence: self.calculate_kg_confidence(&results.kg_concepts), diversity: self.calculate_diversity(&chunks), }; diff --git a/crates/terraphim_grep/tests/synonym_retrieval.rs b/crates/terraphim_grep/tests/synonym_retrieval.rs index 4a078475..190ec7c5 100644 --- a/crates/terraphim_grep/tests/synonym_retrieval.rs +++ b/crates/terraphim_grep/tests/synonym_retrieval.rs @@ -110,3 +110,141 @@ fn synonym_evidence_counts_towards_synthesis_coverage() { "no configured provider must not invent output" ); } + +fn large_synonym_graph(root: &std::path::Path) -> std::path::PathBuf { + let mut data = serde_json::Map::new(); + for name in (0..70) + .map(|index| format!("alias{index:02}")) + .chain(["decision".into(), "zzjudgment".into()]) + { + data.insert(name, serde_json::json!({"id":1,"nterm":"decision"})); + } + let graph = root.join("thesaurus.json"); + std::fs::write( + &graph, + serde_json::to_vec(&serde_json::json!({"name":"large", "data":data})).unwrap(), + ) + .unwrap(); + graph +} + +#[test] +fn retrieval_includes_aliases_beyond_the_presentation_limit() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write(corpus.join("brief.md"), "Alias69: public preview.\n").unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["zzjudgment", "--search-only", "--json", "--thesaurus"]) + .arg(large_synonym_graph(root.path())) + .arg("--paths") + .arg(&corpus) + .output() + .unwrap(); + assert!(output.status.success()); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["chunks"].as_array().unwrap().len(), 1, "{result}"); + assert_eq!(result["chunks"][0]["content"], "Alias69: public preview."); +} + +#[test] +fn synthesis_coverage_is_independent_of_the_displayed_concept_limit() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write( + corpus.join("brief.md"), + "Decision: public preview.\nDecision: keep a backup supplier.\nAlias69: reserve a covered area.\n", + ) + .unwrap(); + let graph = large_synonym_graph(root.path()); + for limit in ["3", "50"] { + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args([ + "zzjudgment", + "--answer", + "--json", + "-n", + limit, + "--thesaurus", + ]) + .arg(&graph) + .arg("--paths") + .arg(&corpus) + .env_remove("OPENROUTER_API_KEY") + .env_remove("OLLAMA_BASE_URL") + .output() + .unwrap(); + assert!(output.status.success()); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["chunks"].as_array().unwrap().len(), 3, "{result}"); + assert!(result["concepts"].as_array().unwrap().len() <= limit.parse().unwrap()); + assert_ne!(result["sufficiency"], "RlmInsufficient", "{result}"); + assert!(result["answer"].is_null()); + assert!( + result["sufficiency_explanation"] + .as_str() + .unwrap() + .contains("no LLM client is configured"), + "{result}" + ); + } +} + +#[test] +fn expanded_alias_and_canonical_text_are_literal_not_query_constraints() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write( + corpus.join("brief.md"), + "Plan: risk /team/ needs a backup supplier.\nPlan: risk type:rust needs a covered area.\nPlan: unrelated risk needs no synonym match.\n", + ) + .unwrap(); + let graph = root.path().join("thesaurus.json"); + std::fs::write( + &graph, + r#"{"name":"literal","data":{"risk /team/":{"id":1,"nterm":"risk /team/"},"judgment":{"id":1,"nterm":"risk /team/"},"risk type:rust":{"id":1,"nterm":"risk /team/"}}}"#, + ) + .unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["judgment", "--search-only", "--json", "--thesaurus"]) + .arg(&graph) + .arg("--paths") + .arg(&corpus) + .output() + .unwrap(); + assert!(output.status.success()); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let chunks = result["chunks"].as_array().unwrap(); + assert_eq!(chunks.len(), 2, "{result}"); + assert!(chunks.iter().any(|c| c["line_start"] == 1)); + assert!(chunks.iter().any(|c| c["line_start"] == 2)); +} + +#[test] +fn compound_user_queries_retain_filters_without_synonym_expansion() { + let root = tempfile::tempdir().unwrap(); + let corpus = root.path().join("corpus"); + std::fs::create_dir(&corpus).unwrap(); + std::fs::write(corpus.join("brief.md"), "Judgment: public preview.\n").unwrap(); + std::fs::write(corpus.join("brief.rs"), "Judgment: internal preview.\n").unwrap(); + std::fs::write( + corpus.join("decision.md"), + "Decision: canonical term only.\n", + ) + .unwrap(); + let graph = root.path().join("thesaurus.json"); + std::fs::write(&graph, r#"{"name":"compound","data":{"decision":{"id":1,"nterm":"decision"},"judgment":{"id":1,"nterm":"decision"}}}"#).unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_terraphim-grep")) + .args(["judgment *.rs", "--search-only", "--json", "--thesaurus"]) + .arg(&graph) + .arg("--paths") + .arg(&corpus) + .output() + .unwrap(); + assert!(output.status.success()); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["chunks"].as_array().unwrap().len(), 1, "{result}"); + assert_eq!(result["chunks"][0]["source"], "brief.rs"); +} From ef5f92c5f9a850df5d4895409dd21c3c86438afb Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Sun, 11 Oct 2026 23:31:08 +0100 Subject: [PATCH 7/9] chore(release): prepare v1.21.19 synonym search binaries --- .github/workflows/ci.yml | 3 +++ Cargo.lock | 16 ++++++++-------- Cargo.toml | 2 +- crates/terraphim_grep/CHANGELOG.md | 17 +++++++++++++++++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97a41e10..a9b89bbe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,6 +106,9 @@ jobs: - run: cargo clippy --workspace --all-targets -- -D warnings - run: cargo build --workspace - run: cargo test --workspace --lib --no-fail-fast + # Exercise the released grep command against real files and exported + # synonym graphs, including Linux-only hostile filesystem names. + - run: cargo test --locked -p terraphim_grep --test synonym_retrieval --test answer_request --test path_guard_hostile_tree # GitHub-side home for the updater integration contracts (managed-mode # receipt/refusal, strict v2 manifest readers, R2 checksum fail-closed). # On Gitea these lanes live in .gitea/workflows/native-ci.yml, which is diff --git a/Cargo.lock b/Cargo.lock index 87486cf7..216c438a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6315,7 +6315,7 @@ dependencies = [ [[package]] name = "terraphim-cli" -version = "1.21.18" +version = "1.21.19" dependencies = [ "anyhow", "assert_cmd", @@ -6356,7 +6356,7 @@ dependencies = [ [[package]] name = "terraphim-session-analyzer" -version = "1.21.18" +version = "1.21.19" dependencies = [ "aho-corasick", "anyhow", @@ -6395,7 +6395,7 @@ dependencies = [ [[package]] name = "terraphim_agent" -version = "1.21.18" +version = "1.21.19" dependencies = [ "ahash", "anyhow", @@ -6580,7 +6580,7 @@ dependencies = [ [[package]] name = "terraphim_grep" -version = "1.21.18" +version = "1.21.19" dependencies = [ "anyhow", "async-trait", @@ -6609,7 +6609,7 @@ dependencies = [ [[package]] name = "terraphim_hooks" -version = "1.21.18" +version = "1.21.19" dependencies = [ "dirs 5.0.1", "serde", @@ -6623,7 +6623,7 @@ dependencies = [ [[package]] name = "terraphim_lsp" -version = "1.21.18" +version = "1.21.19" dependencies = [ "log", "serde", @@ -6650,7 +6650,7 @@ dependencies = [ [[package]] name = "terraphim_mcp_server" -version = "1.21.18" +version = "1.21.19" dependencies = [ "ahash", "anyhow", @@ -6713,7 +6713,7 @@ dependencies = [ [[package]] name = "terraphim_negative_contribution" -version = "1.21.18" +version = "1.21.19" dependencies = [ "log", "terraphim_automata", diff --git a/Cargo.toml b/Cargo.toml index 4a6c42bb..4fae7af8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ members = [ ] [workspace.package] -version = "1.21.18" +version = "1.21.19" edition = "2024" authors = ["Terraphim Team "] documentation = "https://terraphim.ai" diff --git a/crates/terraphim_grep/CHANGELOG.md b/crates/terraphim_grep/CHANGELOG.md index bb975006..a5419b3e 100644 --- a/crates/terraphim_grep/CHANGELOG.md +++ b/crates/terraphim_grep/CHANGELOG.md @@ -2,6 +2,23 @@ All notable changes to terraphim_grep are documented here. +## [1.21.19] - 2026-10-11 + +### Fixed +- Search an exact editor-exported synonym across its complete knowledge-graph + equivalence group. Display limits no longer omit aliases or synthesis evidence. +- Treat graph alternatives as literal text, including slash and filter-looking + words, while preserving filters in compound user queries. +- Honor requested `--answer` synthesis when retrieved evidence is sufficient and + preserve the source's decision/implementation status in the answer. +- Pin fff-search and reject mmap content-cache configuration and unsupported + filesystem names before indexing. Concurrent-writer hazards remain outside + this partial mitigation. + +### Changed +- Batch synonym searches into one scan per root and document the editor graph + export → search-only → optional synthesis workflow. + ## [Unreleased] ### Fixed From 2d1820e81d2a9458d4d5eca012e7d0e0d2accf49 Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Sun, 11 Oct 2026 23:32:48 +0100 Subject: [PATCH 8/9] test(release): bind immutable source contract to v1.21.19 --- tests/test_release_binaries_workflow_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_release_binaries_workflow_contract.py b/tests/test_release_binaries_workflow_contract.py index ace1ec1d..f1dfdd46 100644 --- a/tests/test_release_binaries_workflow_contract.py +++ b/tests/test_release_binaries_workflow_contract.py @@ -176,7 +176,7 @@ def test_preflight_recursively_peels_to_exact_expected_sha(self) -> None: def test_release_uses_checked_in_version_and_never_mutates_source(self) -> None: text = workflow_text() workspace = tomllib.loads((ROOT / "Cargo.toml").read_text()) - self.assertEqual(workspace["workspace"]["package"]["version"], "1.21.18") + self.assertEqual(workspace["workspace"]["package"]["version"], "1.21.19") for forbidden in ( "Set release version", "set_section_version", From df10eca2fe85659aed6816982f1a850f5910f5bd Mon Sep 17 00:00:00 2001 From: Dr Alexander Mikhalev Date: Mon, 12 Oct 2026 00:30:32 +0100 Subject: [PATCH 9/9] chore(arch): pin v1.21.19 signed release archives (Refs #316) --- pkgbuilds/terraphim-clients-bin/.SRCINFO | 18 +++++++++--------- pkgbuilds/terraphim-clients-bin/PKGBUILD | 10 +++++----- tests/test_pkgbuild_release_contract.py | 18 +++++++++--------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkgbuilds/terraphim-clients-bin/.SRCINFO b/pkgbuilds/terraphim-clients-bin/.SRCINFO index 86eb3935..43d4e0f0 100644 --- a/pkgbuilds/terraphim-clients-bin/.SRCINFO +++ b/pkgbuilds/terraphim-clients-bin/.SRCINFO @@ -1,6 +1,6 @@ pkgbase = terraphim-clients-bin pkgdesc = Terraphim AI command-line clients (prebuilt static MUSL binaries) - pkgver = 1.21.18 + pkgver = 1.21.19 pkgrel = 1 url = https://terraphim.ai arch = x86_64 @@ -9,14 +9,14 @@ pkgbase = terraphim-clients-bin license = MIT options = !strip options = !debug - source_x86_64 = terraphim-agent-1.21.18-x86_64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.18/terraphim-agent-1.21.18-x86_64-unknown-linux-musl.tar.gz - source_x86_64 = terraphim-grep-1.21.18-x86_64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.18/terraphim-grep-1.21.18-x86_64-unknown-linux-musl.tar.gz - sha256sums_x86_64 = 767ee6cdaa375f838cf69607a7346cb89195480dbfccc45ecefde7820a89ad4c - sha256sums_x86_64 = 8e463f780e3683e909613a4dd01fb47da36e5736a6a51ea62f36a3ce235b7167 - source_aarch64 = terraphim-agent-1.21.18-aarch64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.18/terraphim-agent-1.21.18-aarch64-unknown-linux-musl.tar.gz - source_aarch64 = terraphim-grep-1.21.18-aarch64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.18/terraphim-grep-1.21.18-aarch64-unknown-linux-musl.tar.gz - sha256sums_aarch64 = 1f4265ad8756378d101d35dc21d0362460cb858d9be99aa7486aafe83e8caa0f - sha256sums_aarch64 = 1b02796312162b59cd0165aa302aa683e8f57b00c72f283ab0bd15d569d3b395 + source_x86_64 = terraphim-agent-1.21.19-x86_64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.19/terraphim-agent-1.21.19-x86_64-unknown-linux-musl.tar.gz + source_x86_64 = terraphim-grep-1.21.19-x86_64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.19/terraphim-grep-1.21.19-x86_64-unknown-linux-musl.tar.gz + sha256sums_x86_64 = 6970a26f1d33cad67a78397c6d18d6ca0b5d56200b7d86be3027199ca5d1d5d9 + sha256sums_x86_64 = d95ae22a7a84c294e700e957ff94ad25195481fce80ea7673a240c6240257220 + source_aarch64 = terraphim-agent-1.21.19-aarch64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.19/terraphim-agent-1.21.19-aarch64-unknown-linux-musl.tar.gz + source_aarch64 = terraphim-grep-1.21.19-aarch64-unknown-linux-musl.tar.gz::https://github.com/terraphim/terraphim-clients/releases/download/v1.21.19/terraphim-grep-1.21.19-aarch64-unknown-linux-musl.tar.gz + sha256sums_aarch64 = 5c763fb2308f3d83d3e83f935bfa77a9c90414ec7567eac275bedbc7c179e103 + sha256sums_aarch64 = 1332b48c1ca67e0d2ffd534f5a112c77ffea5885004cbd1909e7ac0aecd2f086 pkgname = terraphim-agent-bin pkgdesc = Terraphim AI agent CLI (prebuilt static MUSL binary) diff --git a/pkgbuilds/terraphim-clients-bin/PKGBUILD b/pkgbuilds/terraphim-clients-bin/PKGBUILD index f796da66..0697e5f5 100644 --- a/pkgbuilds/terraphim-clients-bin/PKGBUILD +++ b/pkgbuilds/terraphim-clients-bin/PKGBUILD @@ -18,7 +18,7 @@ pkgbase=terraphim-clients-bin pkgname=(terraphim-agent-bin terraphim-grep-bin) -pkgver=1.21.18 +pkgver=1.21.19 pkgrel=1 pkgdesc='Terraphim AI command-line clients (prebuilt static MUSL binaries)' arch=('x86_64' 'aarch64') @@ -45,12 +45,12 @@ source_aarch64=( # SHA256SUMS asset for v${pkgver}. tests/test_pkgbuild_release_contract.py pins # the same values so a version bump cannot silently keep stale digests. sha256sums_x86_64=( - '767ee6cdaa375f838cf69607a7346cb89195480dbfccc45ecefde7820a89ad4c' - '8e463f780e3683e909613a4dd01fb47da36e5736a6a51ea62f36a3ce235b7167' + '6970a26f1d33cad67a78397c6d18d6ca0b5d56200b7d86be3027199ca5d1d5d9' + 'd95ae22a7a84c294e700e957ff94ad25195481fce80ea7673a240c6240257220' ) sha256sums_aarch64=( - '1f4265ad8756378d101d35dc21d0362460cb858d9be99aa7486aafe83e8caa0f' - '1b02796312162b59cd0165aa302aa683e8f57b00c72f283ab0bd15d569d3b395' + '5c763fb2308f3d83d3e83f935bfa77a9c90414ec7567eac275bedbc7c179e103' + '1332b48c1ca67e0d2ffd534f5a112c77ffea5885004cbd1909e7ac0aecd2f086' ) # Prebuilt, already-stripped static binaries: never strip or emit debug info. diff --git a/tests/test_pkgbuild_release_contract.py b/tests/test_pkgbuild_release_contract.py index 016c5bfc..5ebaaf61 100644 --- a/tests/test_pkgbuild_release_contract.py +++ b/tests/test_pkgbuild_release_contract.py @@ -27,12 +27,12 @@ D = chr(36) # dollar sign, kept out of template literals in the test source -# Tracks the last *published* release, not the workspace version: the PKGBUILD -# pins the release asset SHA-256 digests, which only exist once the release is -# sealed. During a release window (workspace bumped, assets not yet -# published) this contract intentionally fails until the follow-up PKGBUILD +# Tracks the sealed immutable release: the PKGBUILD pins the signed archive +# SHA-256 digests from its verified release stage, which can be checked before +# publication. During a release window (workspace bumped, signed assets not +# yet sealed) this contract intentionally fails until the follow-up PKGBUILD # bump lands. -PKGVER = "1.21.18" +PKGVER = "1.21.19" REPO = "terraphim/terraphim-clients" RELEASE_BASE = f"https://github.com/{REPO}/releases/download/v{PKGVER}" @@ -41,21 +41,21 @@ "x86_64": { "terraphim-agent": ( f"terraphim-agent-{PKGVER}-x86_64-unknown-linux-musl.tar.gz", - "767ee6cdaa375f838cf69607a7346cb89195480dbfccc45ecefde7820a89ad4c", + "6970a26f1d33cad67a78397c6d18d6ca0b5d56200b7d86be3027199ca5d1d5d9", ), "terraphim-grep": ( f"terraphim-grep-{PKGVER}-x86_64-unknown-linux-musl.tar.gz", - "8e463f780e3683e909613a4dd01fb47da36e5736a6a51ea62f36a3ce235b7167", + "d95ae22a7a84c294e700e957ff94ad25195481fce80ea7673a240c6240257220", ), }, "aarch64": { "terraphim-agent": ( f"terraphim-agent-{PKGVER}-aarch64-unknown-linux-musl.tar.gz", - "1f4265ad8756378d101d35dc21d0362460cb858d9be99aa7486aafe83e8caa0f", + "5c763fb2308f3d83d3e83f935bfa77a9c90414ec7567eac275bedbc7c179e103", ), "terraphim-grep": ( f"terraphim-grep-{PKGVER}-aarch64-unknown-linux-musl.tar.gz", - "1b02796312162b59cd0165aa302aa683e8f57b00c72f283ab0bd15d569d3b395", + "1332b48c1ca67e0d2ffd534f5a112c77ffea5885004cbd1909e7ac0aecd2f086", ), }, }