fix(volume): preserve compose compatibility and enforce invariants

Author: kvinwang

dstack/crates/dstack-cli/src/main.rs

@@ -420,6 +420,7 @@ async fn cmd_deploy(
         .iter()
         .map(|s| parse_volume(s))
         .collect::<Result<Vec<_>>>()?;
+    dstack_types::validate_verity_volumes(&parsed_volumes).map_err(anyhow::Error::msg)?;
 
     // each --volume declares a measured verity_volumes entry, so the built
     // app-compose (and thus app_id) binds the attested roots.

dstack/crates/dstack-volume/src/bin/dstack-volume.rs

@@ -86,6 +86,7 @@ fn prepare_volumes() -> Result<Vec<VerityVolume>> {
 
 fn mount_all(compose_path: PathBuf) -> Result<()> {
     let compose = read_compose(&compose_path)?;
+    dstack_types::validate_verity_volumes(&compose.verity_volumes).map_err(anyhow::Error::msg)?;
     if compose.verity_volumes.is_empty() {
         return Ok(());
     }

dstack/dstack-types/src/lib.rs

@@ -141,6 +141,29 @@ pub struct VerityVolume {
     pub target: std::path::PathBuf,
 }
 
+/// Reject ambiguous volume declarations before any disk is attached or
+/// activated. A root identifies one logical volume, and a mount target can
+/// only be owned by one volume.
+pub fn validate_verity_volumes(volumes: &[VerityVolume]) -> Result<(), String> {
+    let mut roots = std::collections::HashSet::new();
+    let mut targets = std::collections::HashSet::new();
+    for volume in volumes {
+        if !roots.insert(volume.verity_root) {
+            return Err(format!(
+                "duplicate verity root {}",
+                hex::encode(volume.verity_root)
+            ));
+        }
+        if !targets.insert(&volume.target) {
+            return Err(format!(
+                "duplicate verity volume target {}",
+                volume.target.display()
+            ));
+        }
+    }
+    Ok(())
+}
+
 fn deserialize_absolute_path<'de, D>(deserializer: D) -> Result<std::path::PathBuf, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -155,6 +178,30 @@ where
     Ok(path)
 }
 
+#[cfg(test)]
+mod verity_volume_tests {
+    use super::{validate_verity_volumes, VerityVolume};
+
+    fn volume(root: u8, target: &str) -> VerityVolume {
+        VerityVolume {
+            source: format!("{root}.img"),
+            verity_root: [root; 32],
+            target: target.into(),
+        }
+    }
+
+    #[test]
+    fn rejects_duplicate_roots_and_targets() {
+        assert!(validate_verity_volumes(&[volume(1, "/a"), volume(1, "/b")])
+            .unwrap_err()
+            .contains("duplicate verity root"));
+        assert!(validate_verity_volumes(&[volume(1, "/a"), volume(2, "/a")])
+            .unwrap_err()
+            .contains("duplicate verity volume target"));
+        validate_verity_volumes(&[volume(1, "/a"), volume(2, "/b")]).unwrap();
+    }
+}
+
 /// Canonical source for the policy used when `requirements.gpu_policy` is
 /// absent. Both typed defaults and measurement are derived from this JSON.
 pub const DEFAULT_GPU_POLICY: &str = "{}";

dstack/vmm/src/app.rs

@@ -85,8 +85,6 @@ pub struct PortMapping {
 #[derive(Deserialize, Serialize, Debug, Clone)]
 pub struct VmVolume {
     pub source: String,
-    #[serde(default)]
-    pub read_only: bool,
 }
 
 #[derive(Deserialize, Serialize, Clone, Builder, Debug)]
@@ -1741,11 +1739,9 @@ mod tests {
         manifest.volumes = vec![
             VmVolume {
                 source: "/volumes/a.img".into(),
-                read_only: true,
             },
             VmVolume {
                 source: "/volumes/b.img".into(),
-                read_only: true,
             },
         ];
         let vm_config = make_vm_config(

dstack/vmm/src/app/qemu.rs

@@ -176,7 +176,6 @@ fn virtio_pci_device(device: &str, snp: bool) -> String {
 
 struct PreparedVolume {
     source: String,
-    read_only: bool,
 }
 
 struct PreparedQemuLaunch {
@@ -214,7 +213,6 @@ impl PreparedQemuLaunch {
             .iter()
             .map(|volume| PreparedVolume {
                 source: volume.source.clone(),
-                read_only: volume.read_only,
             })
             .collect();
 
@@ -550,10 +548,10 @@ impl QemuCommandBuilder<'_> {
         // established device order.
         for (index, volume) in self.prepared.volumes.iter().enumerate() {
             let id = format!("vol{index}");
-            let mut drive = format!("file={},if=none,id={id},format=raw", volume.source);
-            if volume.read_only {
-                drive.push_str(",readonly=on");
-            }
+            let drive = format!(
+                "file={},if=none,id={id},format=raw,readonly=on",
+                volume.source
+            );
 
             let device = format!("virtio-blk-pci,drive={id}");
             command
@@ -1048,7 +1046,6 @@ mod tests {
                 networks: vec![],
                 volumes: vec![VmVolume {
                     source: "/does-not-exist/volume.img".into(),
-                    read_only: true,
                 }],
             },
             image: Image {
@@ -1086,7 +1083,6 @@ mod tests {
             networks: vec![config.cvm.networking.clone(), config.cvm.networking.clone()],
             volumes: vec![PreparedVolume {
                 source: "/does-not-exist/volume.img".into(),
-                read_only: true,
             }],
             hugepage_numa_nodes: None,
             gpu_numa_nodes: HashMap::new(),

dstack/vmm/src/main_service.rs

@@ -184,9 +184,9 @@ pub fn create_manifest_from_vm_config(
         Some(gpus) => resolve_gpus_with_config(gpus, cvm_config)?,
         None => GpuConfig::default(),
     };
-    let app_compose: AppCompose = serde_json::from_str(&request.compose_file)
-        .context("invalid app-compose in VM configuration")?;
-    let volumes = resolve_volumes(&app_compose.verity_volumes, cvm_config)?;
+    let verity_volumes = extract_verity_volumes(&request.compose_file)?;
+    dstack_types::validate_verity_volumes(&verity_volumes).map_err(anyhow::Error::msg)?;
+    let volumes = resolve_volumes(&verity_volumes, cvm_config)?;
 
     Ok(Manifest {
         id,
@@ -209,6 +209,19 @@ pub fn create_manifest_from_vm_config(
     })
 }
 
+/// Extract only the field understood by this VMM. Keep every other app-compose
+/// field opaque so newer guest schemas and legacy third-party clients remain
+/// compatible with older VMMs.
+fn extract_verity_volumes(compose: &str) -> Result<Vec<dstack_types::VerityVolume>> {
+    let Ok(compose) = serde_json::from_str::<serde_json::Value>(compose) else {
+        return Ok(vec![]);
+    };
+    let Some(volumes) = compose.get("verity_volumes") else {
+        return Ok(vec![]);
+    };
+    serde_json::from_value(volumes.clone()).context("invalid verity_volumes in app-compose")
+}
+
 /// Resolve requested volumes against `cvm.volumes_dir`. Each `source` must be a
 /// bare file name under that directory; the host attaches the bytes, and the
 /// guest verifies content against the measured `verity_root`.
@@ -229,11 +242,6 @@ fn resolve_volumes(
             let real = resolve_volume_source(&base, &v.source)?;
             Ok(crate::app::VmVolume {
                 source: real.to_string_lossy().into_owned(),
-                // Verity volumes are always read-only: the backing file is shared
-                // content-addressed data, so a writable attach could only let one
-                // guest corrupt it for every other tenant. Force it regardless of
-                // what the client asked for.
-                read_only: true,
             })
         })
         .collect()
@@ -905,8 +913,7 @@ mod tests {
         VmConfiguration {
             name: "vm-test".to_string(),
             image: "dstack-test".to_string(),
-            compose_file: r#"{"manifest_version":2,"name":"test","runner":"docker-compose"}"#
-                .to_string(),
+            compose_file: "{}".to_string(),
             vcpu: 1,
             memory: 1024,
             disk_size: 10,
@@ -934,6 +941,25 @@ mod tests {
         assert!(manifest.networks.is_empty());
     }
 
+    #[test]
+    fn volume_extraction_keeps_other_compose_fields_opaque() -> Result<()> {
+        assert!(extract_verity_volumes("not json")?.is_empty());
+        assert!(extract_verity_volumes(r#"{"future_manifest":true}"#)?.is_empty());
+
+        let compose = serde_json::json!({
+            "unknown_future_field": { "anything": true },
+            "verity_volumes": [{
+                "source": "volume.img",
+                "verity_root": "11".repeat(32),
+                "target": "/run/volume"
+            }]
+        });
+        let volumes = extract_verity_volumes(&compose.to_string())?;
+        assert_eq!(volumes.len(), 1);
+        assert_eq!(volumes[0].verity_root, [0x11; 32]);
+        Ok(())
+    }
+
     #[test]
     fn explicit_user_networking_is_resolved_before_persist() {
         let mut request = test_vm_configuration();
@@ -1009,7 +1035,7 @@ mod tests {
     }
 
     #[test]
-    fn resolve_volumes_forces_read_only() -> Result<()> {
+    fn resolve_volumes_resolves_measured_source() -> Result<()> {
         let tmp = tempfile::tempdir()?;
         fs::write(tmp.path().join("volume.img"), b"volume")?;
         let mut cvm_config = test_cvm_config();
@@ -1025,7 +1051,10 @@ mod tests {
         )?;
 
         assert_eq!(volumes.len(), 1);
-        assert!(volumes[0].read_only);
+        assert_eq!(
+            volumes[0].source,
+            tmp.path().join("volume.img").display().to_string()
+        );
         Ok(())
     }
 }