Improve altool error reporting

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "xcnotary"
-version = "0.4.6"
+version = "0.4.7"
 authors = ["David Vartan <[email protected]>"]
 edition = "2018"
 license = "MIT OR Apache-2.0"

src/notarize/run.rs

@@ -144,16 +144,9 @@ impl NotarizeOp {
         let output = self.run_altool(AltoolArgs::NotarizeApp {
             path: &input_path.path,
             bundle_id: &self.bundle_id,
-        });
-
-        if !output.status.success() {
-            return Err(OperationError::detail(
-                "Notarization upload failed",
-                &String::from_utf8(output.stderr).unwrap(),
-            ));
-        }
+        })?;
 
-        let upload = plist::notarization_upload_response(&output.stdout);
+        let upload = plist::notarization_upload_response(output.as_bytes());
 
         Ok(upload.details.request_uuid)
     }
@@ -162,16 +155,9 @@ impl NotarizeOp {
     fn get_status(&self, request_id: &str) -> Result<NotarizationInfo, OperationError> {
         let output = self.run_altool(AltoolArgs::NotarizationInfo {
             request_id: request_id.clone(),
-        });
-
-        if !output.status.success() {
-            return Err(OperationError::detail(
-                "Notarization status check failed",
-                &String::from_utf8(output.stderr).unwrap(),
-            ));
-        }
+        })?;
 
-        let info = plist::notarization_status_response(&output.stdout);
+        let info = plist::notarization_status_response(output.as_bytes());
 
         if !info
             .success_message
@@ -201,7 +187,7 @@ impl NotarizeOp {
         Ok(())
     }
 
-    fn run_altool(&self, args: AltoolArgs) -> std::process::Output {
+    fn run_altool(&self, args: AltoolArgs) -> Result<String, OperationError> {
         let args = match args {
             AltoolArgs::NotarizationInfo { request_id } => vec!["--notarization-info", &request_id],
             AltoolArgs::NotarizeApp { path, bundle_id } => vec![
@@ -218,7 +204,7 @@ impl NotarizeOp {
             .as_ref()
             .map_or(vec![], |p| vec!["--asc-provider", &p]);
 
-        Command::new("/usr/bin/xcrun")
+        let output = Command::new("/usr/bin/xcrun")
             .args(&[
                 "altool",
                 "-u",
@@ -231,6 +217,18 @@ impl NotarizeOp {
             .args(provider_args)
             .args(args)
             .output()
-            .unwrap()
+            .unwrap();
+
+        let stdout = String::from_utf8(output.stdout).unwrap();
+
+        if output.status.success() {
+            Ok(stdout)
+        } else {
+            let combined = String::from_utf8(output.stderr).unwrap() + &stdout;
+            Err(OperationError::detail(
+                "Notarization service returned an error. Please check the output and try again",
+                &combined,
+            ))
+        }
     }
 }

src/precheck/bundle.rs

@@ -61,7 +61,7 @@ impl super::Precheck for HardenedRuntimeCheck {
         } else {
             Ok(Status::fail_with(
                 "Bundle does not have hardened runtime enabled.",
-                r#"codesign using --runtime flag, or pass OTHER_CODE_SIGN_FLAGS=--runtime to xcodebuild. You can also enable the "Hardened Runtime" capability in Xcode's target settings > "Signing and Capabilities""#,
+                r#"codesign using --options runtime flag, or pass OTHER_CODE_SIGN_FLAGS=--runtime to xcodebuild. You can also enable the "Hardened Runtime" capability in Xcode's target settings > "Signing and Capabilities""#,
                 None,
             ))
         }
@@ -87,12 +87,14 @@ impl super::Precheck for NoGetTaskAllowCheck {
         }
 
         if !output.stdout.is_empty() {
-            if let Some(true) = crate::util::plist::bundle_entitlemens(&output.stdout).get_task_allow {
+            if let Some(true) =
+                crate::util::plist::bundle_entitlemens(&output.stdout).get_task_allow
+            {
                 return Ok(Status::fail_with(
                     "Bundle includes get-task-allow entitlement.",
                     "Specify CODE_SIGN_INJECT_BASE_ENTITLEMENTS=NO when running xcodebuild.",
                     None,
-                ))
+                ));
             }
         }
 

src/util/input_path.rs

@@ -83,9 +83,18 @@ mod tests {
 
     #[test]
     fn test_identify_path() {
-        assert_eq!(Some(PathType::AppBundle), identify_path_type(PathBuf::from("Foo.app")).ok());
-        assert_eq!(Some(PathType::DiskImage), identify_path_type(PathBuf::from("Foo.dmg")).ok());
-        assert_eq!(Some(PathType::InstallerPackage), identify_path_type(PathBuf::from("Foo.pkg")).ok());
+        assert_eq!(
+            Some(PathType::AppBundle),
+            identify_path_type(PathBuf::from("Foo.app")).ok()
+        );
+        assert_eq!(
+            Some(PathType::DiskImage),
+            identify_path_type(PathBuf::from("Foo.dmg")).ok()
+        );
+        assert_eq!(
+            Some(PathType::InstallerPackage),
+            identify_path_type(PathBuf::from("Foo.pkg")).ok()
+        );
         assert!(identify_path_type(PathBuf::from("Foo")).is_err());
     }
 }

src/util/plist/mod.rs

@@ -124,7 +124,7 @@ mod tests {
 </plist>
 "#;
 
-    static ENTITLEMENTS_OUTPUT:&str = r#"
+    static ENTITLEMENTS_OUTPUT: &str = r#"
 <?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">

src/util/plist/structs.rs

@@ -45,7 +45,7 @@ pub(crate) enum NotarizationStatus {
 #[serde(rename_all = "kebab-case")]
 pub(crate) struct BundleEntitlements {
     #[serde(rename = "com.apple.security.get-task-allow")]
-    pub(crate) get_task_allow: Option<bool>
+    pub(crate) get_task_allow: Option<bool>,
 }
 
 /// Response from altool --upload-app