Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down
58 changes: 51 additions & 7 deletions crates/graphforge-storage/src/project_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -100,6 +100,15 @@ pub struct ResolvedProjectGeneration {
manifest_sha256: [u8; 32],
manifest: Arc<GenerationManifest>,
_lease_handle: Arc<GenerationLease>,
/// 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<OnceLock<..>>` 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<OnceLock<Result<Option<Arc<crate::GraphFilesInventory>>, GfError>>>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -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<Option<crate::GraphFilesInventory>, 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<Option<crate::GraphFilesInventory>, GfError> {
let Some(participant) = self.declared_graph_files_participant()? else {
return Ok(None);
};
Expand All @@ -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)?;
Expand All @@ -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,
Expand Down Expand Up @@ -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()),
});
}
}
Expand Down Expand Up @@ -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()),
})
}

Expand Down
Loading