Skip to content

Commit 78e1b90

Browse files
abstractjabstractj
committed
Bot creating duplicated GitHub issues in keycloak-private when receiving e-mails from secalert
Closes #69 Signed-off-by: Bruno Oliveira da Silva <bruno@abstractj.com>
1 parent 294058c commit 78e1b90

6 files changed

Lines changed: 250 additions & 40 deletions

File tree

‎src/main/java/org/keycloak/gh/bot/security/command/SecAlertCommand.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@
1818
import java.util.stream.Collectors;
1919

2020
/**
21-
* Sends emails to SecAlert. Creates a new thread when no SecAlert-Thread-ID exists, otherwise replies on the existing thread.
21+
* Sends emails to SecAlert. Targets secalert.email.to for the initial outreach and secalert.email.reply-to once a
22+
* SecAlert-Thread-ID (captured from the reply address) is recorded on the issue.
2223
*/
23-
@Command(name = "secalert", description = "Sends a new email or reply to SecAlert and tracks the thread ID")
24+
@Command(name = "secalert", description = "Sends a new email or reply to SecAlert and tracks the reply thread ID")
2425
public class SecAlertCommand extends CommandParser implements BotCommand {
2526

2627
private static final Logger LOGGER = Logger.getLogger(SecAlertCommand.class);
@@ -30,8 +31,11 @@ public class SecAlertCommand extends CommandParser implements BotCommand {
3031
@Inject
3132
MailSender mailSender;
3233

33-
@ConfigProperty(name = "email.sender.secalert")
34-
String secAlertEmail;
34+
@ConfigProperty(name = "secalert.email.to")
35+
String secAlertTo;
36+
37+
@ConfigProperty(name = "secalert.email.reply-to")
38+
String secAlertReplyTo;
3539

3640
@ConfigProperty(name = "google.group.target")
3741
String targetGroup;
@@ -59,26 +63,25 @@ private void createNewThread(GHEventPayload.IssueComment payload, String subject
5963
return;
6064
}
6165

62-
LOGGER.infof("Sending new SecAlert email for issue #%d, subject: %s", payload.getIssue().getNumber(), subject);
66+
GHIssue issue = payload.getIssue();
67+
String taggedSubject = subject + " - " + Constants.GHI_ISSUE_PREFIX + issue.getNumber();
68+
69+
LOGGER.infof("Sending new SecAlert email for issue #%d, subject: %s", issue.getNumber(), taggedSubject);
6370

64-
Optional<String> threadId = mailSender.sendNewEmail(secAlertEmail, targetGroup, subject, body);
71+
Optional<String> threadId = mailSender.sendNewEmail(secAlertTo, targetGroup, taggedSubject, body);
6572
if (threadId.isEmpty()) {
6673
fail(payload, "Failed to send email to SecAlert via Gmail API.");
6774
return;
6875
}
6976

70-
String marker = Constants.SECALERT_THREAD_ID_PREFIX + " " + threadId.get();
71-
GHIssue issue = payload.getIssue();
72-
issue.comment("SecAlert email sent. " + marker);
73-
7477
Set<String> currentLabels = issue.getLabels().stream().map(GHLabel::getName).collect(Collectors.toSet());
7578
if (currentLabels.contains(Status.TRIAGE.toLabel())) {
7679
issue.removeLabels(Status.TRIAGE.toLabel());
7780
}
7881
issue.addLabels(Status.CVE_REQUEST.toLabel());
7982

8083
String title = issue.getTitle();
81-
if (!title.startsWith(Constants.CVE_TBD_PREFIX)) {
84+
if (title != null && !title.startsWith(Constants.CVE_TBD_PREFIX)) {
8285
issue.setTitle(Constants.CVE_TBD_PREFIX + " " + title);
8386
}
8487
success(payload);
@@ -87,7 +90,7 @@ private void createNewThread(GHEventPayload.IssueComment payload, String subject
8790
private void replyToExistingThread(GHEventPayload.IssueComment payload, String threadId, String body) throws IOException {
8891
LOGGER.infof("Replying to existing SecAlert thread %s for issue #%d", threadId, payload.getIssue().getNumber());
8992

90-
if (mailSender.sendThreadedEmail(threadId, secAlertEmail, targetGroup, body)) {
93+
if (mailSender.sendThreadedEmail(threadId, secAlertReplyTo, targetGroup, body)) {
9194
success(payload);
9295
} else {
9396
fail(payload, "Failed to send reply to SecAlert thread " + threadId + ".");

‎src/main/java/org/keycloak/gh/bot/security/common/Constants.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ public final class Constants {
66

77
public static final Pattern CVE_PATTERN = Pattern.compile("CVE-\\d{4}-\\d+");
88

9+
public static final String GHI_ISSUE_PREFIX = "#GHI-";
10+
public static final Pattern GHI_ISSUE_PATTERN = Pattern.compile("#GHI-(\\d{1,9})");
11+
912
public static final String GMAIL_THREAD_ID_PREFIX = "**Gmail-Thread-ID:**";
1013
public static final String SECALERT_THREAD_ID_PREFIX = "**SecAlert-Thread-ID:**";
1114
public static final String CVE_TBD_PREFIX = "[CVE-TBD]";

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

Lines changed: 81 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.keycloak.gh.bot.security.common.Constants;
1515
import org.keycloak.gh.bot.utils.Labels;
1616
import org.kohsuke.github.GHIssue;
17+
import org.kohsuke.github.GHIssueComment;
1718
import org.kohsuke.github.GHIssueState;
1819
import org.kohsuke.github.GHLabel;
1920
import org.kohsuke.github.GHRepository;
@@ -23,9 +24,11 @@
2324
import java.time.Duration;
2425
import java.util.List;
2526
import java.util.Map;
27+
import java.util.Objects;
2628
import java.util.Optional;
2729
import java.util.regex.Matcher;
2830
import java.util.regex.Pattern;
31+
import java.util.stream.Stream;
2932

3033
@ApplicationScoped
3134
public class MailProcessor {
@@ -43,8 +46,8 @@ public class MailProcessor {
4346
@ConfigProperty(name = "repository.privateRepository")
4447
String repositoryName;
4548

46-
@ConfigProperty(name = "email.sender.secalert")
47-
String secAlertEmail;
49+
@ConfigProperty(name = "secalert.email.reply-to")
50+
String secAlertReplyTo;
4851

4952
@Inject
5053
GmailAdapter gmail;
@@ -62,6 +65,11 @@ public class MailProcessor {
6265
.expireAfterWrite(Duration.ofDays(7))
6366
.build();
6467

68+
private final Cache<Integer, Boolean> secAlertThreadIdCache = Caffeine.newBuilder()
69+
.maximumSize(500)
70+
.expireAfterWrite(Duration.ofDays(7))
71+
.build();
72+
6573
@PostConstruct
6674
void init() {
6775
this.targetGroup = TargetGroup.from(targetGroupEmail);
@@ -100,15 +108,19 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
100108
var msg = gmail.getMessage(msgSummary.getId());
101109
var headers = gmail.getHeadersMap(msg);
102110
var from = headers.getOrDefault("From", "");
111+
var replyTo = headers.getOrDefault("Reply-To", "");
103112

104-
if (isFromBot(from) || !isValidGroupMessage(headers)) {
113+
boolean fromSecAlert = isFromSecAlert(from, replyTo);
114+
115+
if (isFromBot(from) || (!fromSecAlert && !isValidGroupMessage(headers))) {
105116
gmail.markAsRead(msgSummary.getId());
106117
return;
107118
}
108119

109120
var threadId = msg.getThreadId();
110121
var messageId = headers.getOrDefault("Message-ID", "").replaceAll("^<|>$", "");
111-
var subject = normalizeSubject(headers.getOrDefault("Subject", "(No Subject)").trim());
122+
var rawSubject = headers.getOrDefault("Subject", "(No Subject)").trim();
123+
var subject = normalizeSubject(rawSubject);
112124

113125
var body = bodySanitizer.sanitize(gmail.getBody(msg)).orElse("(No content)");
114126
var attachments = gmail.getAttachments(msg);
@@ -117,8 +129,10 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
117129

118130
var issueOpt = resolveIssue(github, repository, threadId);
119131

120-
if (issueOpt.isEmpty() && isFromSecAlert(from)) {
121-
issueOpt = resolveIssueBySecAlertThread(github, repository, threadId);
132+
if (issueOpt.isEmpty() && fromSecAlert) {
133+
issueOpt = resolveIssueByGhiTag(repository, rawSubject)
134+
.or(() -> resolveIssueBySecAlertThreadId(github, repository, threadId));
135+
issueOpt.ifPresent(issue -> issueCache.put(threadId, issue.getNumber()));
122136
}
123137

124138
if (issueOpt.isPresent()) {
@@ -129,8 +143,9 @@ private void processSingleMessage(Message msgSummary, GitHub github, GHRepositor
129143
}
130144
appendComment(issue, from, body, attachmentSection);
131145

132-
if (isFromSecAlert(from)) {
146+
if (fromSecAlert) {
133147
applyCveIdFromSecAlert(issue, subject, body);
148+
recordSecAlertThreadIdIfMissing(issue, threadId);
134149
}
135150
} else {
136151
var newIssue = createNewIssue(repository, threadId, subject, from, body, attachmentSection);
@@ -218,25 +233,76 @@ private boolean isFromBot(String from) {
218233
return from != null && from.toLowerCase().contains(botEmail.toLowerCase());
219234
}
220235

221-
private boolean isFromSecAlert(String from) {
222-
return from != null && from.toLowerCase().contains(secAlertEmail.toLowerCase());
236+
private boolean isFromSecAlert(String from, String replyTo) {
237+
String needle = secAlertReplyTo.toLowerCase();
238+
return Stream.of(from, replyTo)
239+
.filter(Objects::nonNull)
240+
.anyMatch(header -> header.toLowerCase().contains(needle));
223241
}
224242

225-
private Optional<GHIssue> resolveIssueBySecAlertThread(GitHub github, GHRepository repository, String threadId) {
243+
private Optional<GHIssue> resolveIssueBySecAlertThreadId(GitHub github, GHRepository repository, String threadId) {
226244
try {
227-
var query = "repo:%s \"%s %s\" is:issue in:comments".formatted(
228-
repositoryName, Constants.SECALERT_THREAD_ID_PREFIX, threadId);
245+
var expectedMarker = Constants.SECALERT_THREAD_ID_PREFIX + " " + threadId;
246+
var query = "repo:%s \"%s\" is:issue in:comments".formatted(repositoryName, expectedMarker);
229247
var iterator = github.searchIssues().q(query).list().iterator();
230-
if (iterator.hasNext()) {
248+
while (iterator.hasNext()) {
231249
int issueNumber = iterator.next().getNumber();
232-
return Optional.ofNullable(repository.getIssue(issueNumber));
250+
var issue = repository.getIssue(issueNumber);
251+
if (hasExactSecAlertThreadId(issue, expectedMarker)) {
252+
issueCache.put(threadId, issueNumber);
253+
return Optional.of(issue);
254+
}
233255
}
234256
} catch (Exception e) {
235-
LOGGER.warnf(e, "GitHub search failed for SecAlert thread %s", threadId);
257+
LOGGER.warnf(e, "GitHub search by SecAlert-Thread-ID failed for thread %s", threadId);
236258
}
237259
return Optional.empty();
238260
}
239261

262+
private boolean hasExactSecAlertThreadId(GHIssue issue, String expectedMarker) throws IOException {
263+
for (GHIssueComment c : issue.queryComments().list()) {
264+
String body = c.getBody();
265+
if (body != null && body.trim().equals(expectedMarker)) {
266+
secAlertThreadIdCache.put(issue.getNumber(), Boolean.TRUE);
267+
return true;
268+
}
269+
}
270+
return false;
271+
}
272+
273+
Optional<GHIssue> resolveIssueByGhiTag(GHRepository repository, String subject) {
274+
if (subject == null) return Optional.empty();
275+
Matcher matcher = Constants.GHI_ISSUE_PATTERN.matcher(subject);
276+
if (!matcher.find()) {
277+
return Optional.empty();
278+
}
279+
int issueNumber = Integer.parseInt(matcher.group(1));
280+
try {
281+
return Optional.ofNullable(repository.getIssue(issueNumber));
282+
} catch (IOException e) {
283+
LOGGER.warnf(e, "Failed to fetch issue #%d referenced by SecAlert subject tag", issueNumber);
284+
return Optional.empty();
285+
}
286+
}
287+
288+
void recordSecAlertThreadIdIfMissing(GHIssue issue, String threadId) throws IOException {
289+
if (threadId == null || threadId.isBlank()) return;
290+
int issueNumber = issue.getNumber();
291+
if (Boolean.TRUE.equals(secAlertThreadIdCache.getIfPresent(issueNumber))) {
292+
return;
293+
}
294+
for (GHIssueComment c : issue.queryComments().list()) {
295+
String body = c.getBody();
296+
if (body != null && body.contains(Constants.SECALERT_THREAD_ID_PREFIX)) {
297+
secAlertThreadIdCache.put(issueNumber, Boolean.TRUE);
298+
return;
299+
}
300+
}
301+
issue.comment(Constants.SECALERT_THREAD_ID_PREFIX + " " + threadId);
302+
secAlertThreadIdCache.put(issueNumber, Boolean.TRUE);
303+
LOGGER.infof("Recorded %s %s on issue #%d", Constants.SECALERT_THREAD_ID_PREFIX, threadId, issueNumber);
304+
}
305+
240306
void applyCveIdFromSecAlert(GHIssue issue, String subject, String body) throws IOException {
241307
String cveId = extractCveId(subject);
242308
if (cveId == null) {

‎src/main/resources/application.properties‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ google.group.target=keycloak-security@googlegroups.com
4646
# --- Google Group Target ---
4747
#Sync interval for checking e-mails
4848
bot.email.sync.interval=10m
49-
email.sender.secalert=secalert@redhat.com
49+
secalert.email.to=secalert@redhat.com
50+
secalert.email.reply-to=secalert@redhat.atlassian.net
5051

5152
repository.mainRepository=keycloak/keycloak
5253
# For sensitive data the bot will only process emails and commands if installed on this repository

‎src/test/java/org/keycloak/gh/bot/security/command/SecAlertCommandTest.java‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.concurrent.CountDownLatch;
2424
import java.util.concurrent.TimeUnit;
2525

26+
import static org.junit.jupiter.api.Assertions.assertNull;
2627
import static org.mockito.ArgumentMatchers.*;
2728
import static org.mockito.Mockito.*;
2829

@@ -40,7 +41,8 @@ void setUp() throws Exception {
4041
command = new SecAlertCommand();
4142

4243
setField(command, "mailSender", mailSender);
43-
setField(command, "secAlertEmail", "secalert@redhat.com");
44+
setField(command, "secAlertTo", "secalert@redhat.com");
45+
setField(command, "secAlertReplyTo", "secalert@redhat.atlassian.net");
4446
setField(command, "targetGroup", "keycloak-security@googlegroups.com");
4547

4648
payload = mock(GHEventPayload.IssueComment.class);
@@ -59,21 +61,21 @@ void setUp() throws Exception {
5961
// --- New thread (no existing SecAlert-Thread-ID) ---
6062

6163
@Test
62-
void newThread_sendsEmailWithSubjectAndPostsThreadId() throws Exception {
64+
void newThread_sendsEmailWithGhiTaggedSubject() throws Exception {
6365
when(comment.getBody()).thenReturn("@security secalert CVE-2026-1234 XSS in admin console\n\nPlease triage this vulnerability.");
6466
setupSubject("CVE-2026-1234", "XSS", "in", "admin", "console");
6567
setupIssueComments("Some unrelated comment");
6668
setupIssueLabels(Status.TRIAGE.toLabel());
6769
when(issue.getTitle()).thenReturn("Wildcard Redirect URI vulnerability");
6870
when(mailSender.sendNewEmail("secalert@redhat.com", "keycloak-security@googlegroups.com",
69-
"CVE-2026-1234 XSS in admin console", "Please triage this vulnerability."))
71+
"CVE-2026-1234 XSS in admin console - #GHI-42", "Please triage this vulnerability."))
7072
.thenReturn(Optional.of("19c48d1ecb33de98"));
7173

7274
command.run(payload);
7375

7476
verify(mailSender).sendNewEmail("secalert@redhat.com", "keycloak-security@googlegroups.com",
75-
"CVE-2026-1234 XSS in admin console", "Please triage this vulnerability.");
76-
verify(issue).comment("SecAlert email sent. " + Constants.SECALERT_THREAD_ID_PREFIX + " 19c48d1ecb33de98");
77+
"CVE-2026-1234 XSS in admin console - #GHI-42", "Please triage this vulnerability.");
78+
verify(issue, never()).comment(anyString());
7779
verify(issue).removeLabels(Status.TRIAGE.toLabel());
7880
verify(issue).addLabels(Status.CVE_REQUEST.toLabel());
7981
verify(issue).setTitle("[CVE-TBD] Wildcard Redirect URI vulnerability");
@@ -120,6 +122,7 @@ void newThread_blocksDuplicateWebhookViaInMemoryGuard() throws Exception {
120122
setupSubject("Race", "Condition");
121123
setupIssueComments();
122124
setupIssueLabels(Status.TRIAGE.toLabel());
125+
when(issue.getTitle()).thenReturn("Some title");
123126

124127
CountDownLatch emailBlocked = new CountDownLatch(1);
125128
CountDownLatch secondCallDone = new CountDownLatch(1);
@@ -133,11 +136,13 @@ void newThread_blocksDuplicateWebhookViaInMemoryGuard() throws Exception {
133136
SecAlertCommand secondCommand = new SecAlertCommand();
134137
secondCommand.unparsedArgs = new ArrayList<>(List.of("Race", "Condition"));
135138
setField(secondCommand, "mailSender", mailSender);
136-
setField(secondCommand, "secAlertEmail", "secalert@redhat.com");
139+
setField(secondCommand, "secAlertTo", "secalert@redhat.com");
140+
setField(secondCommand, "secAlertReplyTo", "secalert@redhat.atlassian.net");
137141
setField(secondCommand, "targetGroup", "keycloak-security@googlegroups.com");
138142

143+
var firstThreadError = new java.util.concurrent.atomic.AtomicReference<Throwable>();
139144
Thread firstThread = new Thread(() -> {
140-
try { command.run(payload); } catch (IOException e) { throw new RuntimeException(e); }
145+
try { command.run(payload); } catch (Exception e) { firstThreadError.set(e); }
141146
});
142147
firstThread.start();
143148

@@ -146,6 +151,7 @@ void newThread_blocksDuplicateWebhookViaInMemoryGuard() throws Exception {
146151
secondCallDone.countDown();
147152
firstThread.join(5000);
148153

154+
assertNull(firstThreadError.get(), "First thread threw an exception");
149155
verify(mailSender, times(1)).sendNewEmail(anyString(), anyString(), anyString(), anyString());
150156
}
151157

@@ -176,16 +182,16 @@ void newThread_reactsWithThumbsDownWhenSubjectMissing() throws Exception {
176182
// --- Reply on existing thread (SecAlert-Thread-ID found) ---
177183

178184
@Test
179-
void existingThread_repliesWithoutSubject() throws Exception {
185+
void existingThread_repliesToSecAlertReplyToAddress() throws Exception {
180186
when(comment.getBody()).thenReturn("@security secalert\n\nThis is a follow-up reply.");
181-
setupIssueComments("SecAlert email sent. " + Constants.SECALERT_THREAD_ID_PREFIX + " abc123def456");
182-
when(mailSender.sendThreadedEmail("abc123def456", "secalert@redhat.com",
187+
setupIssueComments(Constants.SECALERT_THREAD_ID_PREFIX + " abc123def456");
188+
when(mailSender.sendThreadedEmail("abc123def456", "secalert@redhat.atlassian.net",
183189
"keycloak-security@googlegroups.com", "This is a follow-up reply."))
184190
.thenReturn(true);
185191

186192
command.run(payload);
187193

188-
verify(mailSender).sendThreadedEmail("abc123def456", "secalert@redhat.com",
194+
verify(mailSender).sendThreadedEmail("abc123def456", "secalert@redhat.atlassian.net",
189195
"keycloak-security@googlegroups.com", "This is a follow-up reply.");
190196
verify(mailSender, never()).sendNewEmail(anyString(), anyString(), anyString(), anyString());
191197
verify(issue, never()).removeLabels(any(String[].class));
@@ -196,7 +202,7 @@ void existingThread_repliesWithoutSubject() throws Exception {
196202
@Test
197203
void existingThread_reactsWithThumbsDownWhenReplyFails() throws Exception {
198204
when(comment.getBody()).thenReturn("@security secalert\n\nFailed reply.");
199-
setupIssueComments("SecAlert email sent. " + Constants.SECALERT_THREAD_ID_PREFIX + " abc123");
205+
setupIssueComments(Constants.SECALERT_THREAD_ID_PREFIX + " abc123");
200206
when(mailSender.sendThreadedEmail(anyString(), anyString(), anyString(), anyString())).thenReturn(false);
201207

202208
command.run(payload);

0 commit comments

Comments
 (0)