diff --git a/crates/graphforge-storage/src/graph_object_store/materialization.rs b/crates/graphforge-storage/src/graph_object_store/materialization.rs index b3608c445..b835f842b 100644 --- a/crates/graphforge-storage/src/graph_object_store/materialization.rs +++ b/crates/graphforge-storage/src/graph_object_store/materialization.rs @@ -169,19 +169,16 @@ fn materialize_from_cas( error, ) })?; - let verified = verify_file_counted( - installed, - &entry.content_sha256, + let verified = verify_hardlinked_materialization( + &installed, + installed_identity, + source_identity, entry.byte_length, &cas.diagnostic_root, ); match verified { - Ok(io) => { - return Ok(MaterializeIoEvidence { - read_bytes: io.bytes, - read_calls: io.calls, - ..MaterializeIoEvidence::default() - }); + Ok(()) => { + return Ok(MaterializeIoEvidence::default()); } Err(error) => { let _ = parent.unlink_child_if_identity(name, installed_identity); @@ -193,6 +190,59 @@ fn materialize_from_cas( Err(validation("materialization path has no final component")) } +/// Authenticate a just-installed hardlink without re-reading its bytes. +/// +/// `bucket.link_child_into` above already confirmed, via `file_identity`, +/// that the newly-created directory entry resolves to the exact same inode +/// as `source` (the CAS object whose bytes were authenticated once already — +/// either by the writer that sealed this object into the CAS, or lazily on +/// first real read via `open_graph_object_by_digest`/ +/// `read_graph_object_by_digest`). A hard link cannot install different +/// bytes than the inode it points at: there is no operation between the +/// link syscall and this check, within the same open, that could have +/// altered them. Re-reading and re-hashing the whole object here (as the +/// deleted `verify_file_counted` call used to) duplicates that +/// authentication for the cost of an O(bytes) sweep, once per materialized +/// object, every open. See #1388 design "Open path redesign" O2. +/// +/// This keeps the checks that stay meaningful after the hardlink: identity +/// (defense in depth — cheap, `stat`-only, even though `link_child_into` +/// already enforced it), declared length (mirrors `verify_file_counted`'s +/// own pre-hash guard, `graph_object_store.rs`'s `verify_file_counted`), and +/// link count (confirms the destination really is a second name for the CAS +/// inode rather than some other single-link file that happened to be +/// substituted in the window between `link_child_into`'s internal checks and +/// this one). +fn verify_hardlinked_materialization( + installed: &File, + installed_identity: graphforge_filesystem::FileIdentity, + source_identity: graphforge_filesystem::FileIdentity, + expected_length: u64, + diagnostic: &Path, +) -> Result<(), GfError> { + if installed_identity != source_identity { + return Err(validation( + "materialized graph object is not the hard-linked CAS object", + )); + } + let metadata = installed + .metadata() + .map_err(|error| storage("inspect materialized graph object", diagnostic, error))?; + if !metadata.is_file() || metadata.len() != expected_length { + return Err(validation( + "materialized graph object length does not match manifest", + )); + } + let link_count = graphforge_filesystem::file_link_count(installed) + .map_err(|error| storage("inspect materialized graph object links", diagnostic, error))?; + if link_count < 2 { + return Err(validation( + "materialized graph object was not hard-linked from the CAS store", + )); + } + Ok(()) +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] struct MaterializeIoEvidence { copied: bool, diff --git a/crates/graphforge-storage/src/project_generation.rs b/crates/graphforge-storage/src/project_generation.rs index 4c49edbbb..18b164b97 100644 --- a/crates/graphforge-storage/src/project_generation.rs +++ b/crates/graphforge-storage/src/project_generation.rs @@ -9,10 +9,10 @@ use std::fmt::Write as _; use std::fs::{File, OpenOptions}; use std::io::Read; use std::path::{Component, Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; #[cfg(windows)] -use std::sync::{Condvar, Mutex, OnceLock}; +use std::sync::{Condvar, Mutex}; use graphforge_core::{GfError, ProjectErrorCode}; use serde::{Deserialize, Serialize}; @@ -100,6 +100,15 @@ pub struct ResolvedProjectGeneration { manifest_sha256: [u8; 32], manifest: Arc, _lease_handle: Arc, + /// Memoized result of [`Self::graph_files_inventory`]. A resolved + /// generation is immutable by construction (`CURRENT` is never + /// re-consulted after resolution, and generations are never mutated in + /// place), so the manifest-authenticated inventory this computes is the + /// same value on every call. `Arc>` lets every clone of this + /// `ResolvedProjectGeneration` — and there are many, one per call site — + /// share one computation instead of independently re-running the full + /// per-entry admission sweep. + inventory_cache: Arc>, GfError>>>, } #[derive(Debug)] @@ -320,6 +329,28 @@ impl ResolvedProjectGeneration { /// Returns structured validation/corruption errors for unsupported /// contracts or inventory/tree mismatch. pub fn graph_files_inventory(&self) -> Result, GfError> { + // Memoized: a resolved generation never mutates and `CURRENT` is not + // re-consulted after resolution (see `inventory_cache`'s doc comment + // on the struct), so this admits the manifest-authenticated + // inventory at most once per generation, regardless of how many + // call sites or clones of this `ResolvedProjectGeneration` ask for + // it within a single open. + match self.inventory_cache.get_or_init(|| { + self.compute_graph_files_inventory() + .map(|opt| opt.map(Arc::new)) + }) { + Ok(Some(inventory)) => Ok(Some((**inventory).clone())), + Ok(None) => Ok(None), + Err(error) => Err(error.clone()), + } + } + + /// Uncached body of [`Self::graph_files_inventory`]. Every call performs + /// the full manifest decode plus, for the V2/mapped-root participant, a + /// presence-and-length check and a full streamed SHA-256 admission per + /// declared graph payload object. Call only through the memoized public + /// method above. + fn compute_graph_files_inventory(&self) -> Result, GfError> { let Some(participant) = self.declared_graph_files_participant()? else { return Ok(None); }; @@ -341,6 +372,22 @@ impl ResolvedProjectGeneration { ) }, )?; + // Presence and declared-length admission only (O(files), one + // `symlink_metadata` per declared object). This deliberately + // does not read or hash payload bytes — see #1388 design + // "Open path redesign" O3: a byte-for-byte SHA-256 sweep over + // every declared object here duplicated the authentication + // every real consumer already performs, lazily, the first + // time it actually reads an object's bytes, via + // `open_graph_object_by_digest`/`read_graph_object_by_digest` + // (`graph_object_store.rs`). Corruption in an object nothing + // in the session reads is no longer caught by open; it + // surfaces on first read, or via the administrative verify + // command. Maintainer-approved tradeoff (#1388): the verify + // command covers whole-store integrity, this path maximises + // usable open speed, and this is an analyst bench, not a + // forensic system. Mirrors the presence-only pattern already + // used by `ShardedCsrIndex::open` (`adjacency.rs`). for entry in &files { let path = crate::graph_object_path(self.container_root(), &entry.content_sha256)?; @@ -355,11 +402,6 @@ impl ResolvedProjectGeneration { "graph payload object length does not match manifest".into(), )); } - crate::verify_graph_object( - self.container_root(), - &entry.content_sha256, - entry.byte_length, - )?; } crate::route_component::authenticate_manifest_routes( root.format_version, @@ -828,6 +870,7 @@ pub fn resolve_project_generation( manifest_sha256: actual_digest, manifest: Arc::new(manifest), _lease_handle: Arc::new(lease), + inventory_cache: Arc::new(OnceLock::new()), }); } } @@ -876,6 +919,7 @@ pub fn resolve_verified_generation( manifest_sha256: actual_digest, manifest: Arc::new(manifest), _lease_handle: Arc::new(lease), + inventory_cache: Arc::new(OnceLock::new()), }) }