fix(oci): write manifest digest to lockfiles (#268)

fetch_feature stored the OCI layer/blob digest into DownloadedFeature.digest,
which both lockfile writers (up's feature-build path and `upgrade`) used to
populate LockfileFeature.resolved/integrity. The reference @devcontainers/cli
expects the manifest digest there, so `devcontainer features
resolve-dependencies` rejected Deacon-generated lockfiles (#264).

Add a manifest_digest field to DownloadedFeature, computed once from the raw
manifest body already fetched in fetch_feature (shared via a new
fetch_manifest_bytes helper so get_manifest/get_manifest_with_digest/
fetch_feature only hit the network once), and populate it on both the
cache-hit and cache-miss return paths. Point both lockfile writers at the new
field; the existing digest field keeps its role as the layer cache key and
blob-integrity check.

Co-authored-by: Claude <claude@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: pofallon

crates/core/src/oci/fetcher.rs

@@ -107,13 +107,30 @@ impl<C: HttpClient> FeatureFetcher<C> {
         info!("Fetching feature: {}", feature_ref.reference());
 
         let result = async {
-            // Get the manifest
-            let manifest = self.get_manifest(feature_ref).await.map_err(|e| match e {
-                crate::errors::DeaconError::Feature(f) => f,
-                _ => FeatureError::Oci {
-                    message: format!("Get manifest error: {}", e),
-                },
-            })?;
+            // Get the raw manifest body once, then derive both the typed
+            // `Manifest` and its `sha256:`-prefixed digest from it (the
+            // digest the reference CLI records in lockfile `resolved`/
+            // `integrity` fields — see #264).
+            let manifest_data =
+                self.fetch_manifest_bytes(feature_ref)
+                    .await
+                    .map_err(|e| match e {
+                        crate::errors::DeaconError::Feature(f) => f,
+                        _ => FeatureError::Oci {
+                            message: format!("Get manifest error: {}", e),
+                        },
+                    })?;
+            let manifest_digest =
+                Self::manifest_digest(feature_ref, &manifest_data).map_err(|e| match e {
+                    crate::errors::DeaconError::Feature(f) => f,
+                    _ => FeatureError::Oci {
+                        message: format!("Manifest digest error: {}", e),
+                    },
+                })?;
+            let manifest: Manifest =
+                serde_json::from_slice(&manifest_data).map_err(|e| FeatureError::Parsing {
+                    message: format!("Failed to parse manifest: {}", e),
+                })?;
             debug!("Got manifest with {} layers", manifest.layers.len());
 
             // For now, assume single tar layer (as per requirements)
@@ -134,7 +151,7 @@ impl<C: HttpClient> FeatureFetcher<C> {
             if is_cached {
                 info!("Found cached feature at: {}", cached_dir.display());
                 let feature = self
-                    .load_cached_feature(cached_dir, layer.digest.clone())
+                    .load_cached_feature(cached_dir, layer.digest.clone(), manifest_digest.clone())
                     .await
                     .map_err(|e| match e {
                         crate::errors::DeaconError::Feature(f) => f,
@@ -190,6 +207,7 @@ impl<C: HttpClient> FeatureFetcher<C> {
                     path: extracted_dir,
                     metadata,
                     digest: layer.digest.clone(),
+                    manifest_digest,
                 },
                 is_cached,
             ))
@@ -244,8 +262,11 @@ impl<C: HttpClient> FeatureFetcher<C> {
         Ok(())
     }
 
-    /// Get the OCI manifest for a feature
-    pub async fn get_manifest(&self, feature_ref: &FeatureRef) -> Result<Manifest> {
+    /// Fetch the raw manifest body for a feature, retrying on transient errors.
+    ///
+    /// Shared by [`Self::get_manifest`], [`Self::get_manifest_with_digest`], and
+    /// [`Self::fetch_feature`] so the manifest is only downloaded once per call site.
+    async fn fetch_manifest_bytes(&self, feature_ref: &FeatureRef) -> Result<Bytes> {
         let manifest_url = format!(
             "https://{}/v2/{}/manifests/{}",
             feature_ref.registry,
@@ -287,6 +308,39 @@ impl<C: HttpClient> FeatureFetcher<C> {
         )
         .await?;
 
+        Ok(manifest_data)
+    }
+
+    /// Compute the `sha256:`-prefixed digest of a raw manifest body, verifying
+    /// it against a digest-pinned reference (e.g. `feature@sha256:...`) if one
+    /// was requested.
+    fn manifest_digest(feature_ref: &FeatureRef, manifest_data: &[u8]) -> Result<String> {
+        let mut hasher = Sha256::new();
+        hasher.update(manifest_data);
+        let digest_hex = format!("{:x}", hasher.finalize());
+
+        // If the reference is digest-pinned (e.g. `feature@sha256:...`, surfaced
+        // here as the tag), the returned manifest MUST hash to that digest.
+        // Otherwise the registry could serve a different manifest than the one
+        // the caller pinned.
+        if let Some(expected_hex) = feature_ref.tag().strip_prefix("sha256:") {
+            if !digest_hex.eq_ignore_ascii_case(expected_hex) {
+                return Err(FeatureError::IntegrityMismatch {
+                    context: format!("manifest for {}", feature_ref.reference()),
+                    expected: format!("sha256:{}", expected_hex),
+                    actual: format!("sha256:{}", digest_hex),
+                }
+                .into());
+            }
+        }
+
+        Ok(format!("sha256:{}", digest_hex))
+    }
+
+    /// Get the OCI manifest for a feature
+    pub async fn get_manifest(&self, feature_ref: &FeatureRef) -> Result<Manifest> {
+        let manifest_data = self.fetch_manifest_bytes(feature_ref).await?;
+
         let manifest: Manifest =
             serde_json::from_slice(&manifest_data).map_err(|e| FeatureError::Parsing {
                 message: format!("Failed to parse manifest: {}", e),
@@ -303,66 +357,15 @@ impl<C: HttpClient> FeatureFetcher<C> {
         &self,
         feature_ref: &FeatureRef,
     ) -> Result<(serde_json::Value, String)> {
-        let manifest_url = format!(
-            "https://{}/v2/{}/manifests/{}",
-            feature_ref.registry,
-            feature_ref.repository(),
-            feature_ref.tag()
-        );
+        let manifest_data = self.fetch_manifest_bytes(feature_ref).await?;
 
-        debug!("Fetching manifest with digest from: {}", manifest_url);
-
-        let mut headers = HashMap::new();
-        headers.insert(
-            "Accept".to_string(),
-            "application/vnd.oci.image.manifest.v1+json".to_string(),
-        );
-
-        // Retry the manifest download with exponential backoff
-        let manifest_data = retry_async(
-            &self.retry_config,
-            || {
-                let client = &self.client;
-                let url = &manifest_url;
-                let headers = headers.clone();
-                async move {
-                    client.get_with_headers(url, headers).await.map_err(|e| {
-                        let error_msg = e.to_string();
-                        if error_msg.contains("Authentication failed") {
-                            FeatureError::Authentication {
-                                message: format!("Failed to authenticate for manifest: {}", e),
-                            }
-                        } else {
-                            FeatureError::Download {
-                                message: format!("Failed to download manifest: {}", e),
-                            }
-                        }
-                    })
-                }
-            },
-            classify_network_error,
-        )
-        .await?;
-
-        // Compute SHA256 digest of the raw manifest body
-        let mut hasher = Sha256::new();
-        hasher.update(&manifest_data);
-        let digest = format!("{:x}", hasher.finalize());
-
-        // If the reference is digest-pinned (e.g. `feature@sha256:...`, surfaced
-        // here as the tag), the returned manifest MUST hash to that digest.
-        // Otherwise the registry could serve a different manifest than the one
-        // the caller pinned.
-        if let Some(expected_hex) = feature_ref.tag().strip_prefix("sha256:") {
-            if !digest.eq_ignore_ascii_case(expected_hex) {
-                return Err(FeatureError::IntegrityMismatch {
-                    context: format!("manifest for {}", feature_ref.reference()),
-                    expected: format!("sha256:{}", expected_hex),
-                    actual: format!("sha256:{}", digest),
-                }
-                .into());
-            }
-        }
+        // Historically this method returned the bare hex digest (no `sha256:`
+        // prefix); preserve that for existing callers computing canonical IDs.
+        let digest_with_prefix = Self::manifest_digest(feature_ref, &manifest_data)?;
+        let digest = digest_with_prefix
+            .strip_prefix("sha256:")
+            .unwrap_or(&digest_with_prefix)
+            .to_string();
 
         // Parse the manifest JSON
         let manifest: serde_json::Value =
@@ -619,6 +622,7 @@ impl<C: HttpClient> FeatureFetcher<C> {
         &self,
         cached_dir: PathBuf,
         digest: String,
+        manifest_digest: String,
     ) -> Result<DownloadedFeature> {
         let metadata_path = cached_dir.join("devcontainer-feature.json");
         // parse_feature_metadata is sync and does file IO; offload to spawn_blocking
@@ -637,6 +641,7 @@ impl<C: HttpClient> FeatureFetcher<C> {
             path: cached_dir,
             metadata,
             digest,
+            manifest_digest,
         })
     }
 

crates/core/src/oci/types.rs

@@ -94,8 +94,15 @@ pub struct DownloadedFeature {
     pub path: PathBuf,
     /// Feature metadata
     pub metadata: FeatureMetadata,
-    /// Feature digest for caching
+    /// Layer (blob) digest, used as the on-disk cache key and for blob
+    /// integrity verification. This is NOT the digest the reference CLI
+    /// expects in lockfile `resolved`/`integrity` fields — use
+    /// `manifest_digest` for that.
     pub digest: String,
+    /// `sha256:`-prefixed digest of the OCI manifest body. This is the value
+    /// the reference `@devcontainers/cli` records in lockfile `resolved`
+    /// (`{registry}/{repository}@{manifest_digest}`) and `integrity` fields.
+    pub manifest_digest: String,
 }
 
 /// Downloaded and extracted template data

crates/core/tests/integration_oci.rs

@@ -108,6 +108,16 @@ async fn test_oci_feature_fetch_with_mock_client() {
     let manifest_url = "https://ghcr.io/v2/test/feature/manifests/1.0.0";
     let blob_url = format!("https://ghcr.io/v2/test/feature/blobs/{}", layer_digest);
 
+    // Expected manifest digest: sha256 of the exact manifest body served over
+    // the wire — this is what the lockfile's `resolved`/`integrity` fields
+    // must carry (#264), distinct from the layer/blob digest above.
+    let expected_manifest_digest = {
+        use sha2::{Digest, Sha256};
+        let mut hasher = Sha256::new();
+        hasher.update(manifest_json.as_bytes());
+        format!("sha256:{:x}", hasher.finalize())
+    };
+
     fetcher
         .client()
         .add_response(manifest_url.to_string(), Bytes::from(manifest_json))
@@ -139,10 +149,22 @@ async fn test_oci_feature_fetch_with_mock_client() {
     );
     assert!(downloaded_feature.path.join("install.sh").exists());
 
+    // #264 guard: manifest_digest is the manifest body digest, NOT the layer
+    // digest, and never silently regresses to it.
+    assert_eq!(downloaded_feature.manifest_digest, expected_manifest_digest);
+    assert_ne!(
+        downloaded_feature.manifest_digest,
+        downloaded_feature.digest
+    );
+    assert_ne!(downloaded_feature.manifest_digest, layer_digest);
+
     // Test caching - fetch the same feature again
     let cached_feature = fetcher.fetch_feature(&feature_ref).await.unwrap();
     assert_eq!(cached_feature.metadata.id, "test-feature");
     assert_eq!(cached_feature.path, downloaded_feature.path);
+    // The cache-hit branch must populate manifest_digest identically to the
+    // cache-miss branch (both construct DownloadedFeature independently).
+    assert_eq!(cached_feature.manifest_digest, expected_manifest_digest);
 }
 
 /// Security regression: a registry that serves blob bytes which do not match
@@ -250,6 +272,7 @@ echo "Installation completed"
         path: feature_dir,
         metadata,
         digest: "test-digest".to_string(),
+        manifest_digest: "sha256:test-manifest-digest".to_string(),
     };
 
     // Create fetcher and test installation
@@ -289,6 +312,7 @@ async fn test_oci_feature_install_no_script() {
         path: feature_dir,
         metadata,
         digest: "test-digest".to_string(),
+        manifest_digest: "sha256:test-manifest-digest".to_string(),
     };
 
     // Create fetcher and test installation

crates/core/tests/integration_redaction.rs

@@ -1159,6 +1159,7 @@ async fn test_oci_install_env_secret_not_in_logs() {
         path: feature_dir,
         metadata,
         digest: "test-digest".to_string(),
+        manifest_digest: "sha256:test-manifest-digest".to_string(),
     };
 
     let client = ReqwestClient::new().unwrap();

crates/deacon/src/commands/up/features_build.rs

@@ -545,7 +545,8 @@ async fn resolve_and_stage_features(
                 DownloadedFeature {
                     path: canonical_path.clone(),
                     metadata,
-                    digest,
+                    digest: digest.clone(),
+                    manifest_digest: digest,
                 },
             );
 
@@ -657,7 +658,8 @@ async fn resolve_and_stage_features(
                     DownloadedFeature {
                         path: canonical_path.clone(),
                         metadata,
-                        digest,
+                        digest: digest.clone(),
+                        manifest_digest: digest,
                     },
                 );
                 let dep_ref = FeatureRef::new(
@@ -944,7 +946,7 @@ fn build_lockfile_from_features(
         let entry = LockfileFeature::from_resolved(
             &feature_ref.registry,
             &feature_ref.repository(),
-            &downloaded.digest,
+            &downloaded.manifest_digest,
             version,
             depends_on,
         );
@@ -1033,6 +1035,7 @@ mod lockfile_assembly_tests {
                 ..FeatureMetadata::default()
             },
             digest: digest.to_string(),
+            manifest_digest: digest.to_string(),
         }
     }
 
@@ -1050,6 +1053,7 @@ mod lockfile_assembly_tests {
                 ..FeatureMetadata::default()
             },
             digest: digest.to_string(),
+            manifest_digest: digest.to_string(),
         }
     }
 
@@ -1101,6 +1105,59 @@ mod lockfile_assembly_tests {
         assert!(entry.depends_on.is_none());
     }
 
+    /// #264 guard: the lockfile writer must record the OCI *manifest* digest,
+    /// not the layer/blob digest used for on-disk caching — even when they
+    /// differ, as they always do in practice.
+    #[test]
+    fn build_lockfile_uses_manifest_digest_not_layer_digest() {
+        let feature_ref = FeatureRef::new(
+            "ghcr.io".to_string(),
+            "devcontainers".to_string(),
+            "python".to_string(),
+            Some("1".to_string()),
+        );
+        let canonical = "ghcr.io/devcontainers/python".to_string();
+        let user_id = "ghcr.io/devcontainers/python:1".to_string();
+
+        let layer_digest =
+            "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string();
+        let manifest_digest =
+            "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string();
+        assert_ne!(layer_digest, manifest_digest);
+
+        let downloaded = DownloadedFeature {
+            path: PathBuf::from("/tmp/unused"),
+            metadata: FeatureMetadata {
+                id: "python".to_string(),
+                version: Some("1.2.3".to_string()),
+                ..FeatureMetadata::default()
+            },
+            digest: layer_digest,
+            manifest_digest: manifest_digest.clone(),
+        };
+
+        let mut downloaded_features = HashMap::new();
+        downloaded_features.insert(canonical.clone(), downloaded);
+        let mut user_id_by_canonical = HashMap::new();
+        user_id_by_canonical.insert(canonical.clone(), user_id.clone());
+
+        let lockfile = build_lockfile_from_features(
+            &[(canonical, feature_ref)],
+            &downloaded_features,
+            &user_id_by_canonical,
+        );
+
+        let entry = lockfile
+            .features
+            .get(&user_id)
+            .expect("lockfile must contain the feature entry");
+        assert_eq!(
+            entry.resolved,
+            format!("ghcr.io/devcontainers/python@{}", manifest_digest)
+        );
+        assert_eq!(entry.integrity, manifest_digest);
+    }
+
     #[test]
     fn build_lockfile_falls_back_to_tag_when_version_missing() {
         // Some features ship without a `version` in their metadata; rather