From eec518eb779979aec216ae085cd91b945bfc8458 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira da Silva Date: Mon, 31 Aug 2026 17:12:27 -0300 Subject: [PATCH] Don't re-open completed issues on email reply When a mailing list reply arrives for a closed issue, check the state reason before re-opening. Issues closed as COMPLETED stay closed with an auto-reply posted to GitHub and sent via email. SecAlert emails are excluded and always re-open. Introduces an extensible auto-reply template system (AutoReplyType enum + properties file + CDI bean). Closes #77 Signed-off-by: Bruno Oliveira da Silva --- .../bot/security/email/AutoReplyMessages.java | 41 +++++++++++++++++++ .../gh/bot/security/email/AutoReplyType.java | 8 ++++ .../gh/bot/security/email/MailProcessor.java | 32 +++++++++++++-- .../email/auto-reply-messages.properties | 9 ++++ .../security/email/AutoReplyMessagesTest.java | 26 ++++++++++++ .../bot/security/email/MailProcessorTest.java | 28 +++++++++++++ 6 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 src/main/java/org/keycloak/gh/bot/security/email/AutoReplyMessages.java create mode 100644 src/main/java/org/keycloak/gh/bot/security/email/AutoReplyType.java create mode 100644 src/main/resources/org/keycloak/gh/bot/security/email/auto-reply-messages.properties create mode 100644 src/test/java/org/keycloak/gh/bot/security/email/AutoReplyMessagesTest.java diff --git a/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyMessages.java b/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyMessages.java new file mode 100644 index 0000000..10f9c9e --- /dev/null +++ b/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyMessages.java @@ -0,0 +1,41 @@ +package org.keycloak.gh.bot.security.email; + +import jakarta.inject.Singleton; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Properties; + +/** Loads and serves auto-reply message templates for email responses. */ +@Singleton +public class AutoReplyMessages { + + private static final String RESOURCE = "auto-reply-messages.properties"; + + private final Properties properties; + + public AutoReplyMessages() { + this.properties = loadProperties(); + } + + public String getMessage(AutoReplyType type) { + String value = properties.getProperty(type.name()); + if (value == null) { + throw new IllegalArgumentException("No auto-reply template for " + type.name()); + } + return value; + } + + private static Properties loadProperties() { + Properties props = new Properties(); + try (InputStream stream = AutoReplyMessages.class.getResourceAsStream(RESOURCE)) { + if (stream == null) { + throw new IllegalStateException(RESOURCE + " not found on classpath"); + } + props.load(stream); + } catch (IOException e) { + throw new IllegalStateException("Failed to load " + RESOURCE, e); + } + return props; + } +} diff --git a/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyType.java b/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyType.java new file mode 100644 index 0000000..d50ea85 --- /dev/null +++ b/src/main/java/org/keycloak/gh/bot/security/email/AutoReplyType.java @@ -0,0 +1,8 @@ +package org.keycloak.gh.bot.security.email; + +/** Types of automated email replies sent by the bot when an action cannot be performed. */ +public enum AutoReplyType { + + ISSUE_RESOLVED + +} diff --git a/src/main/java/org/keycloak/gh/bot/security/email/MailProcessor.java b/src/main/java/org/keycloak/gh/bot/security/email/MailProcessor.java index 25d3c72..a990d19 100644 --- a/src/main/java/org/keycloak/gh/bot/security/email/MailProcessor.java +++ b/src/main/java/org/keycloak/gh/bot/security/email/MailProcessor.java @@ -16,6 +16,7 @@ import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueState; +import org.kohsuke.github.GHIssueStateReason; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GitHub; @@ -56,7 +57,13 @@ public class MailProcessor { GitHubInstallationProvider gitHubInstallationProvider; @Inject - EmailBodySanitizer bodySanitizer; // Extracted parsing logic dependency + AutoReplyMessages autoReplyMessages; + + @Inject + MailSender mailSender; + + @Inject + EmailBodySanitizer bodySanitizer; private TargetGroup targetGroup; @@ -138,9 +145,7 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor if (issueOpt.isPresent()) { var issue = issueOpt.get(); if (issue.getState() == GHIssueState.CLOSED) { - issue.reopen(); - issue.addLabels(Labels.STATUS_TRIAGE, Labels.REOPENED_BY_BOT); - LOGGER.infof("Reopened existing closed issue #%d for thread %s", issue.getNumber(), threadId); + handleClosedIssue(issue, fromSecAlert, threadId); } appendComment(issue, from, body, attachmentSection); @@ -241,6 +246,25 @@ private boolean isFromSecAlert(String from, String replyTo) { .anyMatch(header -> header.toLowerCase().contains(needle)); } + private void handleClosedIssue(GHIssue issue, boolean fromSecAlert, String threadId) throws IOException { + if (shouldReopenClosedIssue(fromSecAlert, issue.getStateReason())) { + issue.reopen(); + issue.addLabels(Labels.STATUS_TRIAGE, Labels.REOPENED_BY_BOT); + LOGGER.infof("Reopened existing closed issue #%d for thread %s", issue.getNumber(), threadId); + return; + } + String reply = autoReplyMessages.getMessage(AutoReplyType.ISSUE_RESOLVED); + issue.comment(reply); + mailSender.sendReply(threadId, reply, targetGroup.email()); + LOGGER.infof("Issue #%d is completed — posted auto-reply and sent email for thread %s", + issue.getNumber(), threadId); + } + + boolean shouldReopenClosedIssue(boolean fromSecAlert, GHIssueStateReason stateReason) { + if (fromSecAlert) return true; + return stateReason != GHIssueStateReason.COMPLETED; + } + private Optional resolveIssueBySecAlertThreadId(GitHub github, GHRepository repository, String threadId) { try { var expectedMarker = Constants.SECALERT_THREAD_ID_PREFIX + " " + threadId; diff --git a/src/main/resources/org/keycloak/gh/bot/security/email/auto-reply-messages.properties b/src/main/resources/org/keycloak/gh/bot/security/email/auto-reply-messages.properties new file mode 100644 index 0000000..302b504 --- /dev/null +++ b/src/main/resources/org/keycloak/gh/bot/security/email/auto-reply-messages.properties @@ -0,0 +1,9 @@ +ISSUE_RESOLVED=\ + Thank you for your message. This security issue has been resolved and the \ + corresponding tracking issue has been closed.\n\ + \n\ + If you believe this requires further attention or have new information, \ + please submit a new report via the Keycloak security mailing list.\n\ + \n\ + Best regards,\n\ + Keycloak Security Team diff --git a/src/test/java/org/keycloak/gh/bot/security/email/AutoReplyMessagesTest.java b/src/test/java/org/keycloak/gh/bot/security/email/AutoReplyMessagesTest.java new file mode 100644 index 0000000..af35ffe --- /dev/null +++ b/src/test/java/org/keycloak/gh/bot/security/email/AutoReplyMessagesTest.java @@ -0,0 +1,26 @@ +package org.keycloak.gh.bot.security.email; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Verifies auto-reply message templates load correctly and contain expected content. */ +public class AutoReplyMessagesTest { + + @Test + void getMessage_returnsNonNullForIssueResolved() { + AutoReplyMessages messages = new AutoReplyMessages(); + String result = messages.getMessage(AutoReplyType.ISSUE_RESOLVED); + assertNotNull(result); + } + + @Test + void getMessage_containsExpectedContent() { + AutoReplyMessages messages = new AutoReplyMessages(); + String result = messages.getMessage(AutoReplyType.ISSUE_RESOLVED); + assertTrue(result.contains("resolved")); + assertTrue(result.contains("Keycloak Security Team")); + } + +} diff --git a/src/test/java/org/keycloak/gh/bot/security/email/MailProcessorTest.java b/src/test/java/org/keycloak/gh/bot/security/email/MailProcessorTest.java index a408513..18bc18b 100644 --- a/src/test/java/org/keycloak/gh/bot/security/email/MailProcessorTest.java +++ b/src/test/java/org/keycloak/gh/bot/security/email/MailProcessorTest.java @@ -9,6 +9,7 @@ import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueCommentQueryBuilder; +import org.kohsuke.github.GHIssueStateReason; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHRepository; import org.kohsuke.github.PagedIterable; @@ -20,6 +21,7 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyString; @@ -276,6 +278,32 @@ void recordSecAlertThreadIdIfMissing_usesCache() throws Exception { verify(issue.queryComments(), org.mockito.Mockito.times(1)).list(); } + // --- shouldReopenClosedIssue --- + + @Test + void shouldReopenClosedIssue_returnsTrueForSecAlert() { + MailProcessor processor = new MailProcessor(); + assertTrue(processor.shouldReopenClosedIssue(true, GHIssueStateReason.COMPLETED)); + } + + @Test + void shouldReopenClosedIssue_returnsFalseForCompleted() { + MailProcessor processor = new MailProcessor(); + assertFalse(processor.shouldReopenClosedIssue(false, GHIssueStateReason.COMPLETED)); + } + + @Test + void shouldReopenClosedIssue_returnsTrueForNotPlanned() { + MailProcessor processor = new MailProcessor(); + assertTrue(processor.shouldReopenClosedIssue(false, GHIssueStateReason.NOT_PLANNED)); + } + + @Test + void shouldReopenClosedIssue_returnsTrueForNull() { + MailProcessor processor = new MailProcessor(); + assertTrue(processor.shouldReopenClosedIssue(false, null)); + } + @SuppressWarnings("unchecked") private void stubIssueComments(GHIssue issue, String... commentBodies) throws IOException { GHIssueCommentQueryBuilder queryBuilder = mock(GHIssueCommentQueryBuilder.class);