From cf4e938c8ed8640a1d4d9c1d3769d0134777374e Mon Sep 17 00:00:00 2001
From: Jack Grigg <thestr4d@gmail.com>
Date: Sun, 7 Jan 2024 18:10:15 +0000
Subject: [PATCH] Expose fatal errors from rage-keygen

Closes str4d/rage#434.
---
 rage/CHANGELOG.md                 |  3 ++
 rage/src/bin/rage-keygen/error.rs | 40 +++++++++++++++++++++++
 rage/src/bin/rage-keygen/main.rs  | 54 ++++++++++++++-----------------
 3 files changed, 68 insertions(+), 29 deletions(-)
 create mode 100644 rage/src/bin/rage-keygen/error.rs

diff --git a/rage/CHANGELOG.md b/rage/CHANGELOG.md
index 828c9c5..12b7305 100644
--- a/rage/CHANGELOG.md
+++ b/rage/CHANGELOG.md
@@ -15,6 +15,9 @@ to 1.0.0 are beta releases.
 ### Fixed
 - OpenSSH private keys passed to `-i/--identity` that contain invalid public
   keys are no longer ignored when encrypting, and instead cause an error.
+- `rage-keygen` now prints fatal errors directly instead of them being hidden
+  behind the `RUST_LOG=error` environment variable. It also now sets its return
+  code appropriately instead of always returning 0.
 
 ## [0.9.2] - 2023-06-12
 ### Changed
diff --git a/rage/src/bin/rage-keygen/error.rs b/rage/src/bin/rage-keygen/error.rs
new file mode 100644
index 0000000..0e038bb
--- /dev/null
+++ b/rage/src/bin/rage-keygen/error.rs
@@ -0,0 +1,40 @@
+use std::fmt;
+use std::io;
+
+macro_rules! wlnfl {
+    ($f:ident, $message_id:literal) => {
+        writeln!($f, "{}", $crate::fl!($message_id))
+    };
+
+    ($f:ident, $message_id:literal, $($args:expr),* $(,)?) => {
+        writeln!($f, "{}", $crate::fl!($message_id, $($args), *))
+    };
+}
+
+pub(crate) enum Error {
+    FailedToOpenOutput(io::Error),
+    FailedToWriteOutput(io::Error),
+}
+
+// Rust only supports `fn main() -> Result<(), E: Debug>`, so we implement `Debug`
+// manually to provide the error output we want.
+impl fmt::Debug for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Error::FailedToOpenOutput(e) => {
+                wlnfl!(f, "err-failed-to-open-output", err = e.to_string())?
+            }
+            Error::FailedToWriteOutput(e) => {
+                wlnfl!(f, "err-failed-to-write-output", err = e.to_string())?
+            }
+        }
+        writeln!(f)?;
+        writeln!(f, "[ {} ]", crate::fl!("err-ux-A"))?;
+        write!(
+            f,
+            "[ {}: https://str4d.xyz/rage/report {} ]",
+            crate::fl!("err-ux-B"),
+            crate::fl!("err-ux-C")
+        )
+    }
+}
diff --git a/rage/src/bin/rage-keygen/main.rs b/rage/src/bin/rage-keygen/main.rs
index 94f6d68..ed4e65a 100644
--- a/rage/src/bin/rage-keygen/main.rs
+++ b/rage/src/bin/rage-keygen/main.rs
@@ -7,10 +7,11 @@ use i18n_embed::{
     DesktopLanguageRequester,
 };
 use lazy_static::lazy_static;
-use log::error;
 use rust_embed::RustEmbed;
 use std::io::Write;
 
+mod error;
+
 #[derive(RustEmbed)]
 #[folder = "i18n"]
 struct Localizations;
@@ -19,6 +20,7 @@ lazy_static! {
     static ref LANGUAGE_LOADER: FluentLanguageLoader = fluent_language_loader!();
 }
 
+#[macro_export]
 macro_rules! fl {
     ($message_id:literal) => {{
         i18n_embed_fl::fl!($crate::LANGUAGE_LOADER, $message_id)
@@ -41,7 +43,7 @@ struct AgeOptions {
     output: Option<String>,
 }
 
-fn main() {
+fn main() -> Result<(), error::Error> {
     env_logger::builder()
         .format_timestamp(None)
         .filter_level(log::LevelFilter::Off)
@@ -59,35 +61,29 @@ fn main() {
 
     if opts.version {
         println!("rage-keygen {}", env!("CARGO_PKG_VERSION"));
-        return;
-    }
+        Ok(())
+    } else {
+        let mut output =
+            file_io::OutputWriter::new(opts.output, file_io::OutputFormat::Text, 0o600, false)
+                .map_err(error::Error::FailedToOpenOutput)?;
 
-    let mut output =
-        match file_io::OutputWriter::new(opts.output, file_io::OutputFormat::Text, 0o600, false) {
-            Ok(output) => output,
-            Err(e) => {
-                error!("{}", fl!("err-failed-to-open-output", err = e.to_string()));
-                return;
+        let sk = age::x25519::Identity::generate();
+        let pk = sk.to_public();
+
+        (|| {
+            if !output.is_terminal() {
+                eprintln!("{}: {}", fl!("tty-pubkey"), pk);
             }
-        };
 
-    let sk = age::x25519::Identity::generate();
-    let pk = sk.to_public();
-
-    if let Err(e) = (|| {
-        if !output.is_terminal() {
-            eprintln!("{}: {}", fl!("tty-pubkey"), pk);
-        }
-
-        writeln!(
-            output,
-            "# {}: {}",
-            fl!("identity-file-created"),
-            chrono::Local::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true)
-        )?;
-        writeln!(output, "# {}: {}", fl!("identity-file-pubkey"), pk)?;
-        writeln!(output, "{}", sk.to_string().expose_secret())
-    })() {
-        error!("{}", fl!("err-failed-to-write-output", err = e.to_string()));
+            writeln!(
+                output,
+                "# {}: {}",
+                fl!("identity-file-created"),
+                chrono::Local::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true)
+            )?;
+            writeln!(output, "# {}: {}", fl!("identity-file-pubkey"), pk)?;
+            writeln!(output, "{}", sk.to_string().expose_secret())
+        })()
+        .map_err(error::Error::FailedToWriteOutput)
     }
 }