libvirt: Only inject STORAGE_OPTS when bind-storage-ro is enabled

crates/integration-tests/src/tests/libvirt_verb.rs

@@ -955,6 +955,116 @@ fn test_libvirt_run_label_functionality() -> Result<()> {
     Ok(())
 }
 
+#[distributed_slice(INTEGRATION_TESTS)]
+static TEST_LIBVIRT_RUN_NO_STORAGE_OPTS_WITHOUT_BIND_STORAGE: IntegrationTest =
+    IntegrationTest::new(
+        "test_libvirt_run_no_storage_opts_without_bind_storage",
+        test_libvirt_run_no_storage_opts_without_bind_storage,
+    );
+
+/// Test that STORAGE_OPTS credentials are NOT injected when --bind-storage-ro is not used
+fn test_libvirt_run_no_storage_opts_without_bind_storage() -> Result<()> {
+    let test_image = get_test_image();
+
+    // Generate unique domain name for this test
+    let domain_name = format!(
+        "test-no-storage-opts-{}",
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_secs()
+    );
+
+    println!(
+        "Testing that STORAGE_OPTS are not injected without --bind-storage-ro for domain: {}",
+        domain_name
+    );
+
+    // Cleanup any existing domain with this name
+    cleanup_domain(&domain_name);
+
+    // Create domain WITHOUT --bind-storage-ro flag
+    println!("Creating libvirt domain without --bind-storage-ro...");
+    let create_output = run_bcvk(&[
+        "libvirt",
+        "run",
+        "--name",
+        &domain_name,
+        "--label",
+        LIBVIRT_INTEGRATION_TEST_LABEL,
+        "--filesystem",
+        "ext4",
+        &test_image,
+    ])
+    .expect("Failed to run libvirt run");
+
+    println!("Create stdout: {}", create_output.stdout);
+    println!("Create stderr: {}", create_output.stderr);
+
+    if !create_output.success() {
+        cleanup_domain(&domain_name);
+        panic!(
+            "Failed to create domain without --bind-storage-ro: {}",
+            create_output.stderr
+        );
+    }
+
+    println!("Successfully created domain: {}", domain_name);
+
+    // Dump the domain XML to verify STORAGE_OPTS credentials are not present
+    println!("Dumping domain XML to verify no STORAGE_OPTS credentials...");
+    let dumpxml_output = Command::new("virsh")
+        .args(&["dumpxml", &domain_name])
+        .output()
+        .expect("Failed to dump domain XML");
+
+    if !dumpxml_output.status.success() {
+        cleanup_domain(&domain_name);
+        let stderr = String::from_utf8_lossy(&dumpxml_output.stderr);
+        panic!("Failed to dump domain XML: {}", stderr);
+    }
+
+    let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout);
+
+    // Verify that the domain XML does NOT contain STORAGE_OPTS related credentials
+    // The bugfix ensures storage_opts_tmpfiles_d_lines() is only added when --bind-storage-ro is true
+    // These credentials appear as SMBIOS entries in the domain XML
+
+    // Check that bcvk-storage-opts is NOT present (this is the systemd unit name)
+    assert!(
+        !domain_xml.contains("bcvk-storage-opts"),
+        "Domain XML should NOT contain bcvk-storage-opts unit when --bind-storage-ro is not used. Found in XML."
+    );
+    println!("✓ Domain XML does not contain bcvk-storage-opts unit reference");
+
+    // Check that STORAGE_OPTS environment variable is NOT present in SMBIOS credentials
+    assert!(
+        !domain_xml.contains("STORAGE_OPTS"),
+        "Domain XML should NOT contain STORAGE_OPTS environment variable when --bind-storage-ro is not used. Found in XML."
+    );
+    println!("✓ Domain XML does not contain STORAGE_OPTS environment variable");
+
+    // Verify that hoststorage virtiofs tag is NOT present
+    assert!(
+        !domain_xml.contains("hoststorage"),
+        "Domain XML should NOT contain hoststorage virtiofs tag when --bind-storage-ro is not used. Found in XML."
+    );
+    println!("✓ Domain XML does not contain hoststorage virtiofs filesystem");
+
+    // Verify that bind-storage-ro metadata is NOT present
+    assert!(
+        !domain_xml.contains("bootc:bind-storage-ro"),
+        "Domain XML should NOT contain bind-storage-ro metadata when flag is not used. Found in XML."
+    );
+    println!("✓ Domain XML does not contain bind-storage-ro metadata");
+
+    // Cleanup domain
+    cleanup_domain(&domain_name);
+
+    println!("✓ Test passed: STORAGE_OPTS credentials are correctly excluded when --bind-storage-ro is not used");
+    Ok(())
+}
+
 #[distributed_slice(INTEGRATION_TESTS)]
 static TEST_LIBVIRT_ERROR_HANDLING: IntegrationTest =
     IntegrationTest::new("test_libvirt_error_handling", test_libvirt_error_handling);

crates/kit/src/libvirt/run.rs

@@ -961,12 +961,6 @@ fn create_libvirt_domain_from_disk(
     // Generate SMBIOS credential for SSH key injection and systemd environment configuration
     // Combine SSH key setup and storage opts for systemd contexts
     let mut tmpfiles_content = crate::credentials::key_to_root_tmpfiles_d(&public_key_content);
-    tmpfiles_content.push_str(&crate::credentials::storage_opts_tmpfiles_d_lines());
-    let encoded = data_encoding::BASE64.encode(tmpfiles_content.as_bytes());
-    let smbios_cred = format!("io.systemd.credential.binary:tmpfiles.extra={encoded}");
-
-    // Generate SMBIOS credentials for storage opts unit (handles /etc/environment for PAM/SSH)
-    let storage_opts_creds = crate::credentials::smbios_creds_for_storage_opts()?;
 
     let memory = opts.resolved_memory_mb()?;
     let cpus = opts.resolved_cpus()?;
@@ -1063,8 +1057,8 @@ fn create_libvirt_domain_from_disk(
         }
     }
 
-    // Collect mount unit SMBIOS credentials and unit names
-    let mut mount_unit_smbios_creds = Vec::new();
+    // Collect SMBIOS credentials and mount unit names
+    let mut smbios_creds = Vec::new();
     let mut mount_unit_names = Vec::new();
 
     // Process bind mounts (read-write and read-only)
@@ -1073,7 +1067,7 @@ fn create_libvirt_domain_from_disk(
         "bcvk-bind-",
         false,
         domain_builder,
-        &mut mount_unit_smbios_creds,
+        &mut smbios_creds,
         &mut mount_unit_names,
     )?;
 
@@ -1082,7 +1076,7 @@ fn create_libvirt_domain_from_disk(
         "bcvk-bind-ro-",
         true,
         domain_builder,
-        &mut mount_unit_smbios_creds,
+        &mut smbios_creds,
         &mut mount_unit_names,
     )?;
 
@@ -1120,8 +1114,15 @@ fn create_libvirt_domain_from_disk(
         let encoded_mount = data_encoding::BASE64.encode(mount_unit_content.as_bytes());
         let mount_cred =
             format!("io.systemd.credential.binary:systemd.extra-unit.{unit_name}={encoded_mount}");
-        mount_unit_smbios_creds.push(mount_cred);
+        smbios_creds.push(mount_cred);
         mount_unit_names.push(unit_name);
+
+        // Add tmpfiles.d lines and systemd service credentials for STORAGE_OPTS
+        tmpfiles_content.push_str(&crate::credentials::storage_opts_tmpfiles_d_lines());
+        smbios_creds.extend(
+            crate::credentials::smbios_creds_for_storage_opts()
+                .context("Failed to generate storage opts credentials")?,
+        );
     }
 
     // Create a single dropin for local-fs.target that wants all mount units
@@ -1133,25 +1134,25 @@ fn create_libvirt_domain_from_disk(
         let dropin_cred = format!(
             "io.systemd.credential.binary:systemd.unit-dropin.local-fs.target~bcvk-mounts={encoded_dropin}"
         );
-        mount_unit_smbios_creds.push(dropin_cred);
+        smbios_creds.push(dropin_cred);
     }
 
-    // Build QEMU args with all SMBIOS credentials
-    let mut qemu_args = vec![
-        "-smbios".to_string(),
-        format!("type=11,value={}", smbios_cred),
-    ];
+    let mut qemu_args = Vec::new();
 
-    // Add storage opts credentials (unit + dropin)
-    for storage_cred in storage_opts_creds {
-        qemu_args.push("-smbios".to_string());
-        qemu_args.push(format!("type=11,value={}", storage_cred));
+    // Build QEMU args with all SMBIOS credentials
+    {
+        let encoded = data_encoding::BASE64.encode(tmpfiles_content.as_bytes());
+        let smbios_cred = format!("io.systemd.credential.binary:tmpfiles.extra={encoded}");
+        qemu_args.extend([
+            "-smbios".to_string(),
+            format!("type=11,value={}", smbios_cred),
+        ]);
     }
 
-    // Add SMBIOS credentials for mount units
-    for mount_cred in mount_unit_smbios_creds {
+    // Add all SMBIOS credentials (mount units, storage opts, etc.)
+    for cred in smbios_creds {
         qemu_args.push("-smbios".to_string());
-        qemu_args.push(format!("type=11,value={}", mount_cred));
+        qemu_args.push(format!("type=11,value={}", cred));
     }
 
     // Add extra SMBIOS credentials from opts