From 3d68f33df784294bf46c6b3d767e76ddffd8e70c Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Wed, 29 May 2024 22:11:23 -0400
Subject: [PATCH] feat: gpg verification error messages are captured in the
 exception

previously these error messages went to stderr
---
 CHANGELOG.md                      |  3 ++-
 lib/mini_portile2/mini_portile.rb | 32 ++++++++++++++++++-------------
 test/test_digest.rb               |  7 +++----
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dd05487..c0f0e54 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,8 @@
 
 #### Added
 
-- When setting the C compiler through the `MiniPortile` constructor, the preferred keyword argument is now `:cc_command`. The original `:gcc_command` is still supported.
+- When setting the C compiler through the `MiniPortile` constructor, the preferred keyword argument is now `:cc_command`. The original `:gcc_command` is still supported. @flavorjones
+- GPG file verification error messages are captured in the raised exception. Previously these errors went to `stderr`. @flavorjones
 
 
 ### 2.8.6 / 2024-04-14
diff --git a/lib/mini_portile2/mini_portile.rb b/lib/mini_portile2/mini_portile.rb
index e5573e0..662a4e4 100644
--- a/lib/mini_portile2/mini_portile.rb
+++ b/lib/mini_portile2/mini_portile.rb
@@ -8,6 +8,7 @@
 require 'cgi'
 require 'rbconfig'
 require 'shellwords'
+require 'open3'
 
 # Monkey patch for Net::HTTP by ruby open-uri fix:
 # https://github.com/ruby/ruby/commit/58835a9
@@ -463,24 +464,29 @@ def verify_file(file)
       gpg_exe = which('gpg2') || which('gpg') || raise("Neither GPG nor GPG2 is installed")
 
       # import the key into our own keyring
-      gpg_status = IO.popen([gpg_exe, "--status-fd", "1", "--no-default-keyring", "--keyring", KEYRING_NAME, "--import"], "w+") do |io|
-        io.write gpg[:key]
-        io.close_write
-        io.read
+      gpg_error = nil
+      gpg_status = Open3.popen3(gpg_exe, "--status-fd", "1", "--no-default-keyring", "--keyring", KEYRING_NAME, "--import") do |gpg_in, gpg_out, gpg_err, _thread|
+        gpg_in.write gpg[:key]
+        gpg_in.close
+        gpg_error = gpg_err.read
+        gpg_out.read
       end
       key_ids = gpg_status.scan(/\[GNUPG:\] IMPORT_OK \d+ (?<key_id>[0-9a-f]+)/i).map(&:first)
-      raise "invalid gpg key provided" if key_ids.empty?
+      raise "invalid gpg key provided:\n#{gpg_error}" if key_ids.empty?
 
-      # verify the signature against our keyring
-      gpg_status = IO.popen([gpg_exe, "--status-fd", "1", "--no-default-keyring", "--keyring", KEYRING_NAME, "--verify", signature_file, file[:local_path]], &:read)
-
-      # remove the key from our keyring
-      key_ids.each do |key_id|
-        IO.popen([gpg_exe, "--batch", "--yes", "--no-default-keyring", "--keyring", KEYRING_NAME, "--delete-keys", key_id], &:read)
-        raise "unable to delete the imported key" unless $?.exitstatus==0
+      begin
+        # verify the signature against our keyring
+        gpg_status, gpg_error, _status = Open3.capture3(gpg_exe, "--status-fd", "1", "--no-default-keyring", "--keyring", KEYRING_NAME, "--verify", signature_file, file[:local_path])
+
+        raise "signature mismatch:\n#{gpg_error}" unless gpg_status.match(/^\[GNUPG:\] VALIDSIG/)
+      ensure
+        # remove the key from our keyring
+        key_ids.each do |key_id|
+          IO.popen([gpg_exe, "--batch", "--yes", "--no-default-keyring", "--keyring", KEYRING_NAME, "--delete-keys", key_id], &:read)
+          raise "unable to delete the imported key" unless $?.exitstatus==0
+        end
       end
 
-      raise "signature mismatch" unless gpg_status.match(/^\[GNUPG:\] VALIDSIG/)
 
     else
       digest = case
diff --git a/test/test_digest.rb b/test/test_digest.rb
index b70b0df..87c873a 100644
--- a/test/test_digest.rb
+++ b/test/test_digest.rb
@@ -131,7 +131,7 @@ def test_with_invalid_gpg_signature
     exception = assert_raises(RuntimeError){
       @recipe.download
     }
-    assert_equal("signature mismatch", exception.message)
+    assert_includes(exception.message, "signature mismatch")
   end
 
   def test_with_invalid_key
@@ -145,7 +145,7 @@ def test_with_invalid_key
       }
     }
     exception = assert_raises(RuntimeError){ @recipe.download }
-    assert_equal("invalid gpg key provided", exception.message)
+    assert_includes(exception.message, "invalid gpg key provided")
   end
 
   def test_with_different_key_than_one_used_to_sign
@@ -209,7 +209,6 @@ def test_with_different_key_than_one_used_to_sign
       }
     }
     exception = assert_raises(RuntimeError){ @recipe.download }
-    assert_equal("signature mismatch", exception.message)
+    assert_includes(exception.message, "signature mismatch")
   end
 end
-