From caf2229cc681152bf50e46723819e782e2b2d389 Mon Sep 17 00:00:00 2001
From: Björn Persson <Bjorn@Rombobjörn.se>
Date: Aug 01 2020 20:25:10 +0000
Subject: [PATCH 1/2] Don't reject unrelated email messages for lacking In-Reply-To.


The test for "reply+" appears intended to select replies to issues and pull requests so that the milter won't mess with unrelated email traffic. If this is correct, then this must be done before any tests that can cause rejection.

---

diff --git a/pagure-milters/comment_email_milter.py b/pagure-milters/comment_email_milter.py
index 23df6e7..7c4bfa7 100644
--- a/pagure-milters/comment_email_milter.py
+++ b/pagure-milters/comment_email_milter.py
@@ -136,29 +136,31 @@ class PagureMilter(Milter.Base):
         else:
             msg = email.message_from_file(self.fp)
 
-        msg_id = msg.get("In-Reply-To", None)
-        if msg_id is None:
-            self.log("No In-Reply-To, keep going")
-            return Milter.REJECT
-
-        # Ensure we don't get extra lines in the message-id
-        msg_id = msg_id.split("\n")[0].strip()
-
-        self.log("msg-ig %s" % msg_id)
         self.log("To %s" % msg["to"])
         self.log("Cc %s" % msg.get("cc"))
         self.log("From %s" % msg["From"])
 
-        # Check the email was sent to the right address
+        # First check whether the message is addressed to this milter.
         email_address = msg["to"]
         if "reply+" in msg.get("cc", ""):
             email_address = msg["cc"]
         if "reply+" not in email_address:
+            # The message is not addressed to this milter so don't touch it.
             self.log(
                 "No valid recipient email found in To/Cc: %s" % email_address
             )
             return Milter.CONTINUE
 
+        msg_id = msg.get("In-Reply-To", None)
+        if msg_id is None:
+            self.log("No In-Reply-To, can't process this message.")
+            return Milter.REJECT
+
+        # Ensure we don't get extra lines in the message-id
+        msg_id = msg_id.split("\n")[0].strip()
+
+        self.log("msg-id %s" % msg_id)
+
         # Ensure the user replied to his/her own notification, not that
         # they are trying to forge their ID into someone else's
         salt = _config.get("SALT_EMAIL")

From 9348a5dda1f912db4d043bcdedb4a72294c166cf Mon Sep 17 00:00:00 2001
From: Björn Persson <Bjorn@Rombobjörn.se>
Date: Aug 01 2020 20:59:03 +0000
Subject: [PATCH 2/2] Let's not process the email we send.


According to the log message, this check is meant as a safeguard to prevent using Pagure's outgoing email as input. That must be done before any tests that can cause rejection.

---

diff --git a/pagure-milters/comment_email_milter.py b/pagure-milters/comment_email_milter.py
index 7c4bfa7..3627d70 100644
--- a/pagure-milters/comment_email_milter.py
+++ b/pagure-milters/comment_email_milter.py
@@ -151,6 +151,10 @@ class PagureMilter(Milter.Base):
             )
             return Milter.CONTINUE
 
+        if msg["From"] and msg["From"] == _config.get("FROM_EMAIL"):
+            self.log("Let's not process the email we send")
+            return Milter.ACCEPT
+
         msg_id = msg.get("In-Reply-To", None)
         if msg_id is None:
             self.log("No In-Reply-To, can't process this message.")
@@ -201,11 +205,6 @@ class PagureMilter(Milter.Base):
             session.remove()
             return Milter.REJECT
 
-        if msg["From"] and msg["From"] == _config.get("FROM_EMAIL"):
-            self.log("Let's not process the email we send")
-            session.remove()
-            return Milter.REJECT
-
         msg_id = clean_item(msg_id)
 
         if msg_id and "-ticket-" in msg_id: