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_agent/tests/ci_guards.rs b/crates/terraphim_agent/tests/ci_guards.rs index 8cca9655..c09aa463 100644 --- a/crates/terraphim_agent/tests/ci_guards.rs +++ b/crates/terraphim_agent/tests/ci_guards.rs @@ -654,3 +654,168 @@ 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, + ); + // 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 \ + `follow_symlinks: false`; the guard reasoning assumes symlinks \ + are not followed and must not rest on an upstream default \ + (Refs #293)", + i + 1, + ); + } + } +} + +/// 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/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 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_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 4b0cd219..9987e564 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,31 @@ fn thesaurus_query_concepts( } } +/// 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 Vec::new(); + }; + let Some(matched) = matches + .into_iter() + .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). /// Mutates `relevance_score` in place so downstream consumers can see the boost reflected /// in the JSON output -- otherwise the ordering would be inexplicable. @@ -215,6 +240,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 @@ -252,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, @@ -274,11 +348,13 @@ impl HybridSearcher { } else { max_results.saturating_mul(5).max(max_results).min(1000) }; + let aliases = exact_query_aliases(&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); + 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 @@ -341,6 +417,7 @@ impl HybridSearcher { async fn search_code( query: &str, + aliases: &[String], limit: usize, search_path: PathBuf, ) -> Result, String> { @@ -351,11 +428,22 @@ 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, 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))?; @@ -368,9 +456,8 @@ impl HybridSearcher { return Ok(vec![]); } - 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, @@ -383,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() @@ -400,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) @@ -407,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 @@ -625,6 +733,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!( + exact_query_aliases("auth AND deployment", &thesaurus), + Vec::::new() + ); + assert!(exact_query_aliases("author", &thesaurus).is_empty()); + assert!(exact_query_aliases("unknown", &thesaurus).is_empty()); + } + #[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 3cd79945..f647b870 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::{ @@ -191,12 +193,25 @@ 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 { + 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, @@ -508,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/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/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![ diff --git a/crates/terraphim_grep/src/sufficiency_judge.rs b/crates/terraphim_grep/src/sufficiency_judge.rs index d12b198e..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_coverage(query, &chunks), + coverage, kg_confidence: self.calculate_kg_confidence(&results.kg_concepts), diversity: self.calculate_diversity(&chunks), }; @@ -98,6 +123,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 +270,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/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_grep/tests/synonym_retrieval.rs b/crates/terraphim_grep/tests/synonym_retrieval.rs new file mode 100644 index 00000000..190ec7c5 --- /dev/null +++ b/crates/terraphim_grep/tests/synonym_retrieval.rs @@ -0,0 +1,250 @@ +//! 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" + ); +} + +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"); +} 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" } diff --git a/crates/terraphim_mcp_server/src/lib.rs b/crates/terraphim_mcp_server/src/lib.rs index 1f169fd0..aef05605 100644 --- a/crates/terraphim_mcp_server/src/lib.rs +++ b/crates/terraphim_mcp_server/src/lib.rs @@ -91,6 +91,273 @@ 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), + } +} + +/// 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 { @@ -1268,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) @@ -1275,7 +1548,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))?; @@ -1355,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"); @@ -1373,7 +1658,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 +1681,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, @@ -1462,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"); @@ -1486,7 +1783,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 +1812,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, 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}" + ); +} 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", ), }, } 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",