Fix email-to-issue sync: clean up comment formatting

src/main/java/org/keycloak/gh/bot/security/command/CommandParser.java

@@ -45,6 +45,7 @@ private boolean isAuthorizedRepository(GHEventPayload.IssueComment payload) {
     protected void success(GHEventPayload.IssueComment payload) throws IOException {
         payload.getComment().createReaction(ReactionContent.PLUS_ONE);
     }
+
     protected void fail(GHEventPayload.IssueComment payload, String reason) throws IOException {
         LOGGER.errorf("Execution aborted: %s", reason);
         payload.getComment().createReaction(ReactionContent.MINUS_ONE);

src/main/java/org/keycloak/gh/bot/security/email/MailProcessor.java

@@ -21,11 +21,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.regex.Pattern;
 
 @ApplicationScoped
 public class MailProcessor {
 
     private static final Logger LOGGER = Logger.getLogger(MailProcessor.class);
+    private static final Pattern BRACKET_PREFIX_PATTERN = Pattern.compile("^.*?\\[.*?]\\s*");
+    private static final Pattern REPLY_PREFIX_PATTERN = Pattern.compile("^(?:\\s*(?:Re|Fwd|Fw)\\s*:\\s*)+", Pattern.CASE_INSENSITIVE);
 
     @ConfigProperty(name = "google.group.target")
     String targetGroupEmail;
@@ -98,7 +101,7 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
 
             var threadId = msg.getThreadId();
             var messageId = headers.getOrDefault("Message-ID", "").replaceAll("^<|>$", "");
-            var subject = headers.getOrDefault("Subject", "(No Subject)").trim();
+            var subject = normalizeSubject(headers.getOrDefault("Subject", "(No Subject)").trim());
 
             var body = bodySanitizer.sanitize(gmail.getBody(msg)).orElse("(No content)");
             var attachments = gmail.getAttachments(msg);
@@ -113,7 +116,7 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
                     issue.reopen();
                     LOGGER.infof("Reopened existing closed issue #%d for thread %s", issue.getNumber(), threadId);
                 }
-                appendComment(issue, from, subject, body, threadId, attachmentSection);
+                appendComment(issue, from, body, attachmentSection);
             } else {
                 var newIssue = createNewIssue(repository, threadId, subject, from, body, attachmentSection);
                 issueCache.put(threadId, newIssue.getNumber());
@@ -129,7 +132,7 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
     private Optional<GHIssue> resolveIssue(GitHub github, GHRepository repository, String threadId) {
         Integer issueNumber = issueCache.get(threadId, id -> {
             try {
-                var query = "repo:%s \"%s\" label:%s is:issue".formatted(repositoryName, id, Labels.SOURCE_EMAIL);
+                var query = "repo:%s \"%s\" label:%s is:issue in:comments".formatted(repositoryName, id, Labels.SOURCE_EMAIL);
                 var iterator = github.searchIssues().q(query).list().iterator();
                 return iterator.hasNext() ? iterator.next().getNumber() : null;
             } catch (Exception e) {
@@ -170,23 +173,32 @@ private void handleProcessingFailure(String messageId, Exception e) {
         LOGGER.errorf(e, "Failure processing message %s. It will remain unread and be retried.", messageId);
     }
 
-    private void appendComment(GHIssue issue, String from, String subject, String body, String threadId, String attachmentSection) throws IOException {
-        issue.comment(formatGitHubComment(threadId, from, subject, body, attachmentSection));
+    private void appendComment(GHIssue issue, String from, String body, String attachmentSection) throws IOException {
+        issue.comment(formatReplyComment(from, body, attachmentSection));
     }
 
     private GHIssue createNewIssue(GHRepository repo, String threadId, String subject, String from, String body, String attachmentSection) throws IOException {
         var issue = repo.createIssue(subject).body(Constants.ISSUE_DESCRIPTION_TEMPLATE).create();
         issue.addLabels(Labels.STATUS_TRIAGE, Labels.SOURCE_EMAIL);
-        issue.comment(formatGitHubComment(threadId, from, subject, body, attachmentSection));
+        issue.comment(formatNewIssueComment(threadId, subject, from, body, attachmentSection));
         return issue;
     }
 
-    private String formatGitHubComment(String threadId, String from, String subject, String body, String attachmentSection) {
-        return "%s %s\nSubject: %s\nFrom: %s\n\n%s%s".formatted(
+    static String formatNewIssueComment(String threadId, String subject, String from, String body, String attachmentSection) {
+        return "%s %s\nSubject: %s\nFrom: %s\n\n---\n\n%s%s".formatted(
                 Constants.GMAIL_THREAD_ID_PREFIX, threadId, subject, from, body, attachmentSection
         );
     }
 
+    static String formatReplyComment(String from, String body, String attachmentSection) {
+        return "From: %s\n\n---\n\n%s%s".formatted(from, body, attachmentSection);
+    }
+
+    static String normalizeSubject(String subject) {
+        String result = BRACKET_PREFIX_PATTERN.matcher(subject).replaceFirst("");
+        return REPLY_PREFIX_PATTERN.matcher(result).replaceFirst("");
+    }
+
     private boolean isFromBot(String from) {
         return from != null && from.toLowerCase().contains(botEmail.toLowerCase());
     }

src/main/java/org/keycloak/gh/bot/security/email/TargetGroup.java

@@ -3,7 +3,9 @@
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 
-/** Encapsulates the Google Group identity, structural validation, and URL generation. */
+/**
+ * Encapsulates the Google Group identity, structural validation, and URL generation.
+ */
 public record TargetGroup(String email, String id, String domain) {
 
     public static TargetGroup from(String emailAddress) {

src/test/java/org/keycloak/gh/bot/security/email/EmailBodySanitizerTest.java

@@ -0,0 +1,98 @@
+package org.keycloak.gh.bot.security.email;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmailBodySanitizerTest {
+
+    private final EmailBodySanitizer sanitizer = new EmailBodySanitizer();
+
+    @ParameterizedTest
+    @NullAndEmptySource
+    @ValueSource(strings = {"   ", "\n\n"})
+    void sanitize_returnsEmptyForBlankInput(String input) {
+        assertTrue(sanitizer.sanitize(input).isEmpty());
+    }
+
+    @Test
+    void sanitize_returnsPlainTextAsIs() {
+        var result = sanitizer.sanitize("This is a vulnerability report.");
+        assertTrue(result.isPresent());
+        assertEquals("This is a vulnerability report.", result.get());
+    }
+
+    @Test
+    void sanitize_stripsGoogleGroupsFooter() {
+        String body = "Important content.\nYou received this message because you are subscribed to the group.";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertEquals("Important content.", result.get());
+    }
+
+    @Test
+    void sanitize_stripsSignatureDivider() {
+        String body = "Important content.\n-- \nJohn Doe\njohn@example.com";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertEquals("Important content.", result.get());
+    }
+
+    @Test
+    void sanitize_wrapsGmailThreadHistoryInDetails() {
+        String body = "New reply here.\n\nOn Mon, Jan 1, 2024 at 10:00 AM Alice wrote:\n> Original message";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertTrue(result.get().startsWith("New reply here."));
+        assertTrue(result.get().contains("<details>"));
+        assertTrue(result.get().contains("Thread history"));
+        assertTrue(result.get().contains("Original message"));
+    }
+
+    @Test
+    void sanitize_wrapsOutlookPlainDividerInDetails() {
+        String body = "Fresh content.\n\n-----Original Message-----\nFrom: bob@example.com";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertTrue(result.get().startsWith("Fresh content."));
+        assertTrue(result.get().contains("<details>"));
+    }
+
+    @Test
+    void sanitize_wrapsOutlookHtmlDividerInDetails() {
+        String body = "Fresh content.\n\n________________________________\nFrom: bob@example.com";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertTrue(result.get().startsWith("Fresh content."));
+        assertTrue(result.get().contains("<details>"));
+    }
+
+    @Test
+    void sanitize_wrapsExchangeHeaderInDetails() {
+        String body = "My reply.\n\nFrom: Alice Smith\nSent: Monday, January 1, 2024\nTo: Bob";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertTrue(result.get().startsWith("My reply."));
+        assertTrue(result.get().contains("<details>"));
+    }
+
+    @Test
+    void sanitize_wrapsQuotedLinesInDetails() {
+        String body = "My reply.\n\n> quoted line one\n> quoted line two";
+        var result = sanitizer.sanitize(body);
+        assertTrue(result.isPresent());
+        assertTrue(result.get().startsWith("My reply."));
+        assertTrue(result.get().contains("<details>"));
+        assertTrue(result.get().contains("> quoted line one"));
+    }
+
+    @Test
+    void sanitize_returnsEmptyWhenOnlySignature() {
+        String body = "-- \nJohn Doe";
+        assertTrue(sanitizer.sanitize(body).isEmpty());
+    }
+}

src/test/java/org/keycloak/gh/bot/security/email/GmailAdapterTest.java

@@ -0,0 +1,156 @@
+package org.keycloak.gh.bot.security.email;
+
+import com.google.api.services.gmail.model.Message;
+import com.google.api.services.gmail.model.MessagePart;
+import com.google.api.services.gmail.model.MessagePartBody;
+import com.google.api.services.gmail.model.MessagePartHeader;
+import org.junit.jupiter.api.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class GmailAdapterTest {
+
+    private final GmailAdapter adapter = new GmailAdapter();
+
+    @Test
+    void getHeadersMap_extractsHeadersFromMessage() {
+        var msg = messageWithHeaders(
+                header("From", "alice@example.com"),
+                header("Subject", "CVE report"),
+                header("List-ID", "<keycloak-security.googlegroups.com>")
+        );
+
+        var headers = adapter.getHeadersMap(msg);
+        assertEquals("alice@example.com", headers.get("From"));
+        assertEquals("CVE report", headers.get("Subject"));
+    }
+
+    @Test
+    void getHeadersMap_isCaseInsensitive() {
+        var msg = messageWithHeaders(header("From", "alice@example.com"));
+        var headers = adapter.getHeadersMap(msg);
+        assertEquals("alice@example.com", headers.get("from"));
+        assertEquals("alice@example.com", headers.get("FROM"));
+    }
+
+    @Test
+    void getHeadersMap_keepsFirstValueOnDuplicateHeaders() {
+        var msg = messageWithHeaders(
+                header("From", "first@example.com"),
+                header("From", "second@example.com")
+        );
+        assertEquals("first@example.com", adapter.getHeadersMap(msg).get("From"));
+    }
+
+    @Test
+    void getBody_extractsDirectPayloadBody() {
+        var msg = new Message().setPayload(
+                new MessagePart().setBody(new MessagePartBody().setData(encode("Hello world")))
+        );
+        assertEquals("Hello world", adapter.getBody(msg));
+    }
+
+    @Test
+    void getBody_prefersPlainTextOverHtml() {
+        var htmlPart = part("text/html", "<b>Hello</b>");
+        var plainPart = part("text/plain", "Hello plain");
+
+        var msg = new Message().setPayload(
+                new MessagePart().setParts(List.of(htmlPart, plainPart))
+        );
+        assertEquals("Hello plain", adapter.getBody(msg));
+    }
+
+    @Test
+    void getBody_fallsBackToHtmlWhenNoPlainText() {
+        var htmlPart = part("text/html", "<b>Hello</b>");
+
+        var msg = new Message().setPayload(
+                new MessagePart().setParts(List.of(htmlPart))
+        );
+        assertEquals("<b>Hello</b>", adapter.getBody(msg));
+    }
+
+    @Test
+    void getBody_extractsFromNestedParts() {
+        var nested = part("text/plain", "Nested content");
+        var wrapper = new MessagePart().setMimeType("multipart/alternative").setParts(List.of(nested));
+
+        var msg = new Message().setPayload(new MessagePart().setParts(List.of(wrapper)));
+        assertEquals("Nested content", adapter.getBody(msg));
+    }
+
+    @Test
+    void getAttachments_collectsAttachmentsWithIds() {
+        var attachment = new MessagePart()
+                .setFilename("report.pdf")
+                .setMimeType("application/pdf")
+                .setBody(new MessagePartBody().setAttachmentId("att-1"));
+        var textPart = part("text/plain", "body");
+
+        var msg = new Message().setPayload(new MessagePart().setParts(List.of(textPart, attachment)));
+        var result = adapter.getAttachments(msg);
+
+        assertEquals(1, result.size());
+        assertEquals("report.pdf", result.get(0).fileName());
+        assertEquals("application/pdf", result.get(0).mimeType());
+    }
+
+    @Test
+    void getAttachments_ignoresPartsWithoutAttachmentId() {
+        var inline = new MessagePart()
+                .setFilename("image.png")
+                .setMimeType("image/png")
+                .setBody(new MessagePartBody());
+
+        var msg = new Message().setPayload(new MessagePart().setParts(List.of(inline)));
+        assertTrue(adapter.getAttachments(msg).isEmpty());
+    }
+
+    @Test
+    void getAttachments_ignoresPartsWithBlankFilename() {
+        var noName = new MessagePart()
+                .setFilename("")
+                .setMimeType("application/octet-stream")
+                .setBody(new MessagePartBody().setAttachmentId("att-1"));
+
+        var msg = new Message().setPayload(new MessagePart().setParts(List.of(noName)));
+        assertTrue(adapter.getAttachments(msg).isEmpty());
+    }
+
+    @Test
+    void getAttachments_collectsNestedAttachments() {
+        var deepAttachment = new MessagePart()
+                .setFilename("deep.zip")
+                .setMimeType("application/zip")
+                .setBody(new MessagePartBody().setAttachmentId("att-deep"));
+        var wrapper = new MessagePart().setMimeType("multipart/mixed").setParts(List.of(deepAttachment));
+
+        var msg = new Message().setPayload(new MessagePart().setParts(List.of(wrapper)));
+        assertEquals(1, adapter.getAttachments(msg).size());
+        assertEquals("deep.zip", adapter.getAttachments(msg).get(0).fileName());
+    }
+
+    private static Message messageWithHeaders(MessagePartHeader... headers) {
+        return new Message().setPayload(new MessagePart().setHeaders(List.of(headers)));
+    }
+
+    private static MessagePartHeader header(String name, String value) {
+        return new MessagePartHeader().setName(name).setValue(value);
+    }
+
+    private static MessagePart part(String mimeType, String content) {
+        return new MessagePart()
+                .setMimeType(mimeType)
+                .setBody(new MessagePartBody().setData(encode(content)));
+    }
+
+    private static String encode(String text) {
+        return Base64.getUrlEncoder().encodeToString(text.getBytes(StandardCharsets.UTF_8));
+    }
+}