fix(schema): avoid panics in manifest canonicalization

Make NormalizedManifest::canonical_json and compute_env_id fallible and propagate serialization errors through core engine code.
This commit is contained in:
Marco Allegretti 2026-02-25 12:59:19 +01:00
parent c1e2650617
commit a9c09a369e
3 changed files with 18 additions and 17 deletions

View file

@ -112,10 +112,10 @@ impl Engine {
// Use preliminary identity from manifest (not resolved yet). // Use preliminary identity from manifest (not resolved yet).
// This is sufficient for the Defined state; build will re-resolve. // This is sufficient for the Defined state; build will re-resolve.
let identity = compute_env_id(&normalized); let identity = compute_env_id(&normalized)?;
if !self.meta_store.exists(&identity.env_id) { if !self.meta_store.exists(&identity.env_id) {
let manifest_json = normalized.canonical_json(); let manifest_json = normalized.canonical_json()?;
let manifest_hash = self.obj_store.put(manifest_json.as_bytes())?; let manifest_hash = self.obj_store.put(manifest_json.as_bytes())?;
let now = chrono::Utc::now().to_rfc3339(); let now = chrono::Utc::now().to_rfc3339();
@ -225,7 +225,7 @@ impl Engine {
// Phase 1: Resolve dependencies through the backend. // Phase 1: Resolve dependencies through the backend.
// This downloads the base image, computes its content digest, // This downloads the base image, computes its content digest,
// and queries the package manager for exact versions. // and queries the package manager for exact versions.
let preliminary_id = compute_env_id(&normalized); let preliminary_id = compute_env_id(&normalized)?;
let preliminary_spec = RuntimeSpec { let preliminary_spec = RuntimeSpec {
env_id: preliminary_id.env_id.to_string(), env_id: preliminary_id.env_id.to_string(),
root_path: self root_path: self
@ -272,7 +272,8 @@ impl Engine {
); );
// Phase 3: Build the environment, then capture real filesystem layers. // Phase 3: Build the environment, then capture real filesystem layers.
let manifest_hash = self.obj_store.put(normalized.canonical_json().as_bytes())?; let manifest_json = normalized.canonical_json()?;
let manifest_hash = self.obj_store.put(manifest_json.as_bytes())?;
let env_dir = self.layout.env_path(&identity.env_id); let env_dir = self.layout.env_path(&identity.env_id);
@ -639,7 +640,7 @@ impl Engine {
if old_env_ids.is_empty() { if old_env_ids.is_empty() {
let manifest = parse_manifest_file(manifest_path)?; let manifest = parse_manifest_file(manifest_path)?;
let normalized = manifest.normalize()?; let normalized = manifest.normalize()?;
let identity = compute_env_id(&normalized); let identity = compute_env_id(&normalized)?;
if self.meta_store.exists(&identity.env_id) { if self.meta_store.exists(&identity.env_id) {
old_env_ids.push(identity.env_id.to_string()); old_env_ids.push(identity.env_id.to_string());
} }
@ -953,7 +954,7 @@ impl Engine {
) -> Result<(ManifestV1, NormalizedManifest, EnvIdentity), CoreError> { ) -> Result<(ManifestV1, NormalizedManifest, EnvIdentity), CoreError> {
let manifest = parse_manifest_file(manifest_path)?; let manifest = parse_manifest_file(manifest_path)?;
let normalized = manifest.normalize()?; let normalized = manifest.normalize()?;
let identity = compute_env_id(&normalized); let identity = compute_env_id(&normalized)?;
Ok((manifest, normalized, identity)) Ok((manifest, normalized, identity))
} }
} }

View file

@ -22,10 +22,10 @@ pub struct EnvIdentity {
/// - Internal lookup during rebuild (to find old environments) /// - Internal lookup during rebuild (to find old environments)
/// ///
/// After `build`, the env_id stored in metadata comes from the lock file. /// After `build`, the env_id stored in metadata comes from the lock file.
pub fn compute_env_id(normalized: &NormalizedManifest) -> EnvIdentity { pub fn compute_env_id(normalized: &NormalizedManifest) -> Result<EnvIdentity, serde_json::Error> {
let mut hasher = blake3::Hasher::new(); let mut hasher = blake3::Hasher::new();
hasher.update(normalized.canonical_json().as_bytes()); hasher.update(normalized.canonical_json()?.as_bytes());
let base_digest = blake3::hash(normalized.base_image.as_bytes()) let base_digest = blake3::hash(normalized.base_image.as_bytes())
.to_hex() .to_hex()
@ -71,10 +71,10 @@ pub fn compute_env_id(normalized: &NormalizedManifest) -> EnvIdentity {
let hex = hasher.finalize().to_hex().to_string(); let hex = hasher.finalize().to_hex().to_string();
let short = hex[..12].to_owned(); let short = hex[..12].to_owned();
EnvIdentity { Ok(EnvIdentity {
env_id: EnvId::new(hex), env_id: EnvId::new(hex),
short_id: ShortId::new(short), short_id: ShortId::new(short),
} })
} }
#[cfg(test)] #[cfg(test)]
@ -110,7 +110,7 @@ packages = ["clang", "git"]
.normalize() .normalize()
.unwrap(); .unwrap();
assert_eq!(compute_env_id(&a), compute_env_id(&b)); assert_eq!(compute_env_id(&a).unwrap(), compute_env_id(&b).unwrap());
} }
#[test] #[test]
@ -141,7 +141,7 @@ packages = ["git", "cmake"]
.normalize() .normalize()
.unwrap(); .unwrap();
assert_ne!(compute_env_id(&a), compute_env_id(&b)); assert_ne!(compute_env_id(&a).unwrap(), compute_env_id(&b).unwrap());
} }
#[test] #[test]
@ -172,7 +172,7 @@ backend = "oci"
.normalize() .normalize()
.unwrap(); .unwrap();
assert_ne!(compute_env_id(&a), compute_env_id(&b)); assert_ne!(compute_env_id(&a).unwrap(), compute_env_id(&b).unwrap());
} }
#[test] #[test]
@ -188,7 +188,7 @@ image = "rolling"
.normalize() .normalize()
.unwrap(); .unwrap();
let id = compute_env_id(&n); let id = compute_env_id(&n).unwrap();
assert_eq!(id.short_id.as_str().len(), 12); assert_eq!(id.short_id.as_str().len(), 12);
assert!(id.env_id.as_str().starts_with(id.short_id.as_str())); assert!(id.env_id.as_str().starts_with(id.short_id.as_str()));
} }

View file

@ -74,8 +74,8 @@ impl ManifestV1 {
} }
impl NormalizedManifest { impl NormalizedManifest {
pub fn canonical_json(&self) -> String { pub fn canonical_json(&self) -> Result<String, serde_json::Error> {
serde_json::to_string(self).expect("normalized manifest serialization is infallible") serde_json::to_string(self)
} }
} }
@ -175,7 +175,7 @@ packages = ["clang", "git"]
.normalize() .normalize()
.unwrap(); .unwrap();
assert_eq!(a.canonical_json(), b.canonical_json()); assert_eq!(a.canonical_json().unwrap(), b.canonical_json().unwrap());
} }
#[test] #[test]