From fd149eef47ad613c621833b84b8452cd953ec512 Mon Sep 17 00:00:00 2001
From: Ana Hobden <operator@hoverbear.org>
Date: Wed, 1 Feb 2023 10:40:42 -0800
Subject: [PATCH] Verify the apfs volume doesn't already exist before trying to
 create it (#217)

* Verify the apfs volume doesn't already exist before trying to create it

* Use plist instead of raw parsing

* Remove an unwrap
---
 src/action/darwin/create_apfs_volume.rs | 41 +++++++++++++++++++++++--
 src/action/darwin/enable_ownership.rs   |  2 +-
 src/action/darwin/mod.rs                |  2 +-
 src/action/mod.rs                       |  3 ++
 src/settings.rs                         |  1 +
 5 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/src/action/darwin/create_apfs_volume.rs b/src/action/darwin/create_apfs_volume.rs
index 45eb1af..738d193 100644
--- a/src/action/darwin/create_apfs_volume.rs
+++ b/src/action/darwin/create_apfs_volume.rs
@@ -5,6 +5,7 @@ use tracing::{span, Span};
 
 use crate::action::{ActionError, StatefulAction};
 use crate::execute_command;
+use serde::Deserialize;
 
 use crate::action::{Action, ActionDescription};
 
@@ -22,6 +23,22 @@ impl CreateApfsVolume {
         name: String,
         case_sensitive: bool,
     ) -> Result<StatefulAction<Self>, ActionError> {
+        let output =
+            execute_command(Command::new("/usr/sbin/diskutil").args(["apfs", "list", "-plist"]))
+                .await
+                .map_err(ActionError::Command)?;
+
+        let parsed: DiskUtilApfsListOutput = plist::from_bytes(&output.stdout)?;
+        for container in parsed.containers {
+            for volume in container.volumes {
+                if volume.name == name {
+                    return Err(ActionError::Custom(Box::new(
+                        CreateApfsVolumeError::ExistingVolume(name),
+                    )));
+                }
+            }
+        }
+
         Ok(Self {
             disk: disk.as_ref().to_path_buf(),
             name,
@@ -120,7 +137,25 @@ impl Action for CreateApfsVolume {
 }
 
 #[derive(Debug, thiserror::Error)]
-pub enum CreateVolumeError {
-    #[error("Failed to execute command")]
-    Command(#[source] std::io::Error),
+pub enum CreateApfsVolumeError {
+    #[error("Existing volume called `{0}` found in `diskutil apfs list`, delete it with `diskutil apfs deleteVolume \"{0}\"`")]
+    ExistingVolume(String),
+}
+
+#[derive(Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+struct DiskUtilApfsListOutput {
+    containers: Vec<DiskUtilApfsContainer>,
+}
+
+#[derive(Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+struct DiskUtilApfsContainer {
+    volumes: Vec<DiskUtilApfsListVolume>,
+}
+
+#[derive(Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+struct DiskUtilApfsListVolume {
+    name: String,
 }
diff --git a/src/action/darwin/enable_ownership.rs b/src/action/darwin/enable_ownership.rs
index d719038..9c0f700 100644
--- a/src/action/darwin/enable_ownership.rs
+++ b/src/action/darwin/enable_ownership.rs
@@ -62,7 +62,7 @@ impl Action for EnableOwnership {
             .await
             .map_err(ActionError::Command)?
             .stdout;
-            let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf)).unwrap();
+            let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf))?;
 
             the_plist.global_permissions_enabled
         };
diff --git a/src/action/darwin/mod.rs b/src/action/darwin/mod.rs
index 822b6a2..add755a 100644
--- a/src/action/darwin/mod.rs
+++ b/src/action/darwin/mod.rs
@@ -12,7 +12,7 @@ pub(crate) mod kickstart_launchctl_service;
 pub(crate) mod unmount_apfs_volume;
 
 pub use bootstrap_apfs_volume::{BootstrapApfsVolume, BootstrapVolumeError};
-pub use create_apfs_volume::{CreateApfsVolume, CreateVolumeError};
+pub use create_apfs_volume::{CreateApfsVolume, CreateApfsVolumeError};
 pub use create_nix_volume::{CreateNixVolume, NIX_VOLUME_MOUNTD_DEST};
 pub use create_synthetic_objects::{CreateSyntheticObjects, CreateSyntheticObjectsError};
 pub use enable_ownership::{EnableOwnership, EnableOwnershipError};
diff --git a/src/action/mod.rs b/src/action/mod.rs
index 252ef2e..1c1de5a 100644
--- a/src/action/mod.rs
+++ b/src/action/mod.rs
@@ -327,6 +327,9 @@ pub enum ActionError {
         #[from]
         std::string::FromUtf8Error,
     ),
+    /// A MacOS (Darwin) plist related error
+    #[error(transparent)]
+    Plist(#[from] plist::Error),
 }
 
 impl HasExpectedErrors for ActionError {
diff --git a/src/settings.rs b/src/settings.rs
index 474d09e..966a215 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -114,6 +114,7 @@ pub struct CommonSettings {
         feature = "cli",
         clap(long, env = "NIX_INSTALLER_NIX_BUILD_USER_ID_BASE", global = true)
     )]
+    // Service users on Mac should be between 200-400
     #[cfg_attr(all(target_os = "macos", feature = "cli"), clap(default_value_t = 300))]
     #[cfg_attr(
         all(target_os = "linux", feature = "cli"),