From 590d626ea339117578f8a2f143c34172e88ba76c Mon Sep 17 00:00:00 2001 From: suhasagg Date: Sat, 10 Oct 2026 21:25:41 +0530 Subject: [PATCH 1/3] fix(snapshot): serialize concurrent OCI index updates Signed-off-by: suhasagg --- Cargo.lock | 11 ++++ src/hyperlight_host/Cargo.toml | 1 + .../src/sandbox/snapshot/file/mod.rs | 26 +++++++-- .../src/sandbox/snapshot/file_tests.rs | 56 +++++++++++++++++++ 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b78932f0..f5ebb9b4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1143,6 +1143,16 @@ dependencies = [ "pe-unwind-info", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "futures-channel" version = "0.3.34" @@ -1765,6 +1775,7 @@ dependencies = [ "fallible-iterator", "flatbuffers", "framehop", + "fs2", "gdbstub", "gdbstub_arch", "goblin", diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 0d990979e..cc82dfed3 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -21,6 +21,7 @@ bench = false # see https://bheisler.github.io/criterion.rs/book/faq.html#cargo- workspace = true [dependencies] +fs2 = "0.4" gdbstub = { version = "0.7.10", optional = true } gdbstub_arch = { version = "0.3.3", optional = true } goblin = { version = "0.10", default-features = false, features = ["std", "elf32", "elf64", "endian_fd"] } diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index b336907f5..533b42203 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -11,8 +11,11 @@ mod media_types; pub(crate) mod reference; mod transport; +use std::fs::OpenOptions; use std::path::{Path, PathBuf}; +use fs2::FileExt; + use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; use hyperlight_common::vmem::PAGE_SIZE; use oci_spec::image::{ @@ -397,10 +400,25 @@ impl Snapshot { } } - // Validate any pre-existing `oci-layout` marker before - // touching anything else, so a foreign layout (future - // version, hand-edited file) is reported without altering - // the directory. + // Serialize layout validation and the index read-modify-write + // transaction across cooperating processes. + // Use a stable lock file, not index.json, which is atomically replaced. + let lock_path = path.join(".hyperlight-index.lock"); + let lock_file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(&lock_path) + .map_err(|e| { + crate::new_error!("save: failed to open index lock {:?}: {}", lock_path, e) + })?; + lock_file.lock_exclusive().map_err(|e| { + crate::new_error!("save: failed to acquire index lock {:?}: {}", lock_path, e) + })?; + // File lock is released when lock_file is dropped on all exit paths. + + // Validate the existing marker while holding the lock. + // Invalid layouts are rejected before writing snapshot data. let layout_marker = path.join("oci-layout"); let marker_existed = layout_marker .try_exists() diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index d36b4b277..c4174e95c 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -189,6 +189,62 @@ fn snapshot_metadata_namespaces_are_independent() { // Round-trip via OCI layout on disk. +#[test] +fn concurrent_snapshot_saves_preserve_all_tags() { + use std::collections::HashSet; + use std::sync::Barrier; + use std::thread; + + const WRITERS: usize = 8; + + let snapshot = create_snapshot(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("concurrent"); + let barrier = Arc::new(Barrier::new(WRITERS)); + + let handles: Vec<_> = (0..WRITERS) + .map(|i| { + let snapshot = Arc::clone(&snapshot); + let barrier = Arc::clone(&barrier); + let path = path.clone(); + + thread::spawn(move || { + let tag = format!("writer-{i}"); + barrier.wait(); + snapshot.save(&path, &OciTag::new(&tag).unwrap()).unwrap(); + }) + }) + .collect(); + + for handle in handles { + handle.join().unwrap(); + } + + let index: Value = + serde_json::from_slice(&std::fs::read(path.join("index.json")).unwrap()).unwrap(); + + let tags: HashSet = index["manifests"] + .as_array() + .unwrap() + .iter() + .map(|manifest| { + manifest["annotations"]["org.opencontainers.image.ref.name"] + .as_str() + .unwrap() + .to_owned() + }) + .collect(); + + assert_eq!(tags.len(), WRITERS); + + for i in 0..WRITERS { + assert!( + tags.contains(&format!("writer-{i}")), + "missing snapshot tag writer-{i}" + ); + } +} + #[test] fn round_trip_save_load_call() { let snapshot = create_snapshot(); From f23aeb5f85bbd7807aa2f383a93bf6a57abcf9de Mon Sep 17 00:00:00 2001 From: suhasagg Date: Tue, 13 Oct 2026 13:15:41 +0530 Subject: [PATCH 2/3] fix(snapshot): narrow index lock scope and use std file locking Signed-off-by: suhasagg --- Cargo.lock | 11 ---- src/hyperlight_host/Cargo.toml | 1 - .../src/sandbox/snapshot/file/mod.rs | 62 ++++++++----------- .../src/sandbox/snapshot/file_tests.rs | 23 ++++++- 4 files changed, 47 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5ebb9b4f..7b78932f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1143,16 +1143,6 @@ dependencies = [ "pe-unwind-info", ] -[[package]] -name = "fs2" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "futures-channel" version = "0.3.34" @@ -1775,7 +1765,6 @@ dependencies = [ "fallible-iterator", "flatbuffers", "framehop", - "fs2", "gdbstub", "gdbstub_arch", "goblin", diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index cc82dfed3..0d990979e 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -21,7 +21,6 @@ bench = false # see https://bheisler.github.io/criterion.rs/book/faq.html#cargo- workspace = true [dependencies] -fs2 = "0.4" gdbstub = { version = "0.7.10", optional = true } gdbstub_arch = { version = "0.3.3", optional = true } goblin = { version = "0.10", default-features = false, features = ["std", "elf32", "elf64", "endian_fd"] } diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 533b42203..ab0b6646d 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -14,8 +14,6 @@ mod transport; use std::fs::OpenOptions; use std::path::{Path, PathBuf}; -use fs2::FileExt; - use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; use hyperlight_common::vmem::PAGE_SIZE; use oci_spec::image::{ @@ -149,6 +147,22 @@ fn read_layout_marker(path: &Path) -> crate::Result<()> { Ok(()) } + +/// Validate an existing OCI layout marker, if present. +/// Returns whether the marker already exists. +fn validate_existing_layout(path: &Path) -> crate::Result { + let marker = path.join("oci-layout"); + let exists = marker + .try_exists() + .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", marker, e))?; + + if exists { + read_layout_marker(path)?; + } + + Ok(exists) +} + fn load_manifest( path: &Path, blobs_dir: &Path, @@ -400,6 +414,14 @@ impl Snapshot { } } + // Reject unsupported layouts before writing snapshot data. + validate_existing_layout(path)?; + let layout_marker = path.join("oci-layout"); + + // Generate content-addressed snapshot blobs outside the index lock. + let new_desc = self.write_blobs_and_build_descriptor(path, tag, &cfg, &cfg_bytes)?; + let written_digest = OciDigest::from_oci_spec_digest(new_desc.digest()); + // Serialize layout validation and the index read-modify-write // transaction across cooperating processes. // Use a stable lock file, not index.json, which is atomically replaced. @@ -412,40 +434,13 @@ impl Snapshot { .map_err(|e| { crate::new_error!("save: failed to open index lock {:?}: {}", lock_path, e) })?; - lock_file.lock_exclusive().map_err(|e| { + lock_file.lock().map_err(|e| { crate::new_error!("save: failed to acquire index lock {:?}: {}", lock_path, e) })?; // File lock is released when lock_file is dropped on all exit paths. - // Validate the existing marker while holding the lock. - // Invalid layouts are rejected before writing snapshot data. - let layout_marker = path.join("oci-layout"); - let marker_existed = layout_marker - .try_exists() - .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", layout_marker, e))?; - if marker_existed { - let bytes = read_bounded(&layout_marker, MAX_JSON_BLOB_SIZE).map_err(|e| { - crate::new_error!("save: failed to read existing oci-layout: {}", e) - })?; - let v: serde_json::Value = serde_json::from_slice(&bytes).map_err(|e| { - crate::new_error!("save: existing oci-layout is not valid JSON: {}", e) - })?; - match v.get("imageLayoutVersion").and_then(|s| s.as_str()) { - Some(s) if s == OCI_LAYOUT_VERSION => {} - Some(other) => { - return Err(crate::new_error!( - "save: existing imageLayoutVersion {:?} is unsupported (expected {:?})", - other, - OCI_LAYOUT_VERSION - )); - } - None => { - return Err(crate::new_error!( - "save: existing oci-layout is missing imageLayoutVersion" - )); - } - } - } + // Revalidate under the lock in case the layout changed. + let marker_existed = validate_existing_layout(path)?; let index_path = path.join("index.json"); let index_existed = index_path @@ -466,9 +461,6 @@ impl Snapshot { Vec::new() }; - let new_desc = self.write_blobs_and_build_descriptor(path, tag, &cfg, &cfg_bytes)?; - let written_digest = OciDigest::from_oci_spec_digest(new_desc.digest()); - // Replacement is by tag, not by digest: a new snapshot may // hash to a different value but still claim the same logical // ref. Blobs from the replaced manifest become orphans. diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index c4174e95c..0662af6a3 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -1597,6 +1597,15 @@ fn save_rejects_unsupported_existing_layout_version() { br#"{"imageLayoutVersion":"99.0.0"}"#, ) .unwrap(); + // Capture every existing entry and its bytes before the failed save. + let before: Vec<_> = std::fs::read_dir(&path) + .unwrap() + .map(|entry| { + let entry = entry.unwrap(); + (entry.file_name(), std::fs::read(entry.path()).unwrap()) + }) + .collect(); + let err = snap .save(&path, &OciTag::new("latest").unwrap()) .unwrap_err(); @@ -1605,9 +1614,17 @@ fn save_rejects_unsupported_existing_layout_version() { msg.contains("imageLayoutVersion") || msg.contains("unsupported"), "expected unsupported-version error, got: {msg}" ); - assert!( - !path.join("index.json").exists(), - "save must not have written index.json" + let after: Vec<_> = std::fs::read_dir(&path) + .unwrap() + .map(|entry| { + let entry = entry.unwrap(); + (entry.file_name(), std::fs::read(entry.path()).unwrap()) + }) + .collect(); + + assert_eq!( + before, after, + "unsupported layout must leave directory contents unchanged" ); } From d9f2f3a69918439c2f8e630a9db25bbae375d91c Mon Sep 17 00:00:00 2001 From: suhasagg Date: Wed, 28 Oct 2026 16:12:42 +0530 Subject: [PATCH 3/3] fix(snapshot): validate existing OCI index before blob writes Signed-off-by: suhasagg --- .../src/sandbox/snapshot/file/mod.rs | 65 +++++++++++++------ .../src/sandbox/snapshot/file_tests.rs | 35 ++++++++++ 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index ab0b6646d..98aafd9c9 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -147,7 +147,6 @@ fn read_layout_marker(path: &Path) -> crate::Result<()> { Ok(()) } - /// Validate an existing OCI layout marker, if present. /// Returns whether the marker already exists. fn validate_existing_layout(path: &Path) -> crate::Result { @@ -163,6 +162,47 @@ fn validate_existing_layout(path: &Path) -> crate::Result { Ok(exists) } +/// Validate and read an existing OCI index without modifying the layout. +/// Called before writing blobs and again under the index lock. +fn read_existing_index_manifests(index_path: &Path) -> crate::Result> { + if !index_path + .try_exists() + .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", index_path, e))? + { + return Ok(Vec::new()); + } + + let bytes = read_bounded(index_path, MAX_JSON_BLOB_SIZE) + .map_err(|e| crate::new_error!("save: failed to read existing index.json: {}", e))?; + + let existing: ImageIndex = serde_json::from_slice(&bytes).map_err(|e| { + crate::new_error!( + "save: existing index.json is not a valid OCI image index: {}", + e + ) + })?; + + if existing.schema_version() != SCHEMA_VERSION { + return Err(crate::new_error!( + "save: unsupported OCI index schemaVersion {} (expected {})", + existing.schema_version(), + SCHEMA_VERSION + )); + } + + if let Some(media_type) = existing.media_type() + && !matches!(media_type, MediaType::ImageIndex) + { + return Err(crate::new_error!( + "save: OCI index has unexpected media type {} (expected {})", + media_type, + MediaType::ImageIndex + )); + } + + Ok(existing.manifests().to_vec()) +} + fn load_manifest( path: &Path, blobs_dir: &Path, @@ -414,8 +454,10 @@ impl Snapshot { } } - // Reject unsupported layouts before writing snapshot data. + // Validate layout and index before writing snapshot blobs. validate_existing_layout(path)?; + let index_path = path.join("index.json"); + read_existing_index_manifests(&index_path)?; let layout_marker = path.join("oci-layout"); // Generate content-addressed snapshot blobs outside the index lock. @@ -442,24 +484,7 @@ impl Snapshot { // Revalidate under the lock in case the layout changed. let marker_existed = validate_existing_layout(path)?; - let index_path = path.join("index.json"); - let index_existed = index_path - .try_exists() - .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", index_path, e))?; - let mut manifests: Vec = if index_existed { - let bytes = read_bounded(&index_path, MAX_JSON_BLOB_SIZE).map_err(|e| { - crate::new_error!("save: failed to read existing index.json: {}", e) - })?; - let existing: ImageIndex = serde_json::from_slice(&bytes).map_err(|e| { - crate::new_error!( - "save: existing index.json is not a valid OCI image index: {}", - e - ) - })?; - existing.manifests().to_vec() - } else { - Vec::new() - }; + let mut manifests = read_existing_index_manifests(&index_path)?; // Replacement is by tag, not by digest: a new snapshot may // hash to a different value but still claim the same logical diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 0662af6a3..68f8ea3a3 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -189,6 +189,41 @@ fn snapshot_metadata_namespaces_are_independent() { // Round-trip via OCI layout on disk. +#[test] +fn save_rejects_malformed_existing_index_without_writing_blobs() { + let snap = create_snapshot(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("malformed-index"); + + std::fs::create_dir(&path).unwrap(); + + std::fs::write( + path.join("oci-layout"), + br#"{"imageLayoutVersion":"1.0.0"}"#, + ) + .unwrap(); + + std::fs::write(path.join("index.json"), b"not-json").unwrap(); + + let err = snap + .save(&path, &OciTag::new("latest").unwrap()) + .unwrap_err(); + + assert!(err.to_string().contains("index.json")); + + assert_eq!(std::fs::read(path.join("index.json")).unwrap(), b"not-json"); + + assert!( + !path.join("blobs").exists(), + "save wrote snapshot blobs before validating the index" + ); + + assert!( + !path.join(".hyperlight-index.lock").exists(), + "save created a lock file before validating the index" + ); +} + #[test] fn concurrent_snapshot_saves_preserve_all_tags() { use std::collections::HashSet;