1625: Validate input from /summary command

Reviewed-by: erikj

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCheckIssueVisitor.java

@@ -149,7 +149,7 @@ public void visit(TooFewReviewersIssue e) {
     @Override
     public void visit(InvalidReviewersIssue e) {
         var invalid = String.join(", ", e.invalid());
-        throw new IllegalStateException("Invalid reviewers " + invalid);
+        addFailureMessage(e.check(), "Invalid reviewers " + invalid);
     }
 
     @Override
@@ -236,7 +236,8 @@ public void visit(WhitespaceIssue e) {
     @Override
     public void visit(MessageIssue issue) {
         var message = String.join("\n", issue.commit().message());
-        throw new IllegalStateException("Incorrectly formatted commit message: " + message);
+        log.warning("Incorrectly formatted commit message: " + message);
+        addFailureMessage(issue.check(), "Incorrectly formatted commit message");
     }
 
     @Override

bots/pr/src/main/java/org/openjdk/skara/bots/pr/SummaryCommand.java

@@ -28,9 +28,11 @@
 import java.io.PrintWriter;
 import java.nio.file.Path;
 import java.util.*;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 public class SummaryCommand implements CommandHandler {
+    private static final Pattern INVALID_SUMMARY_PATTERN = Pattern.compile("(^(Co-authored-by:)(.*))|(^(Reviewed-by:)(.*))|(^(Backport-of:)(.*))|(^[0-9]+:(.*))");
     @Override
     public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
         if (!command.user().equals(pr.author())) {
@@ -50,17 +52,28 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
             var summary = command.args().lines()
                                  .map(String::strip)
                                  .collect(Collectors.joining("\n"));
-            var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
-            if (summary.contains("\n")) {
-                reply.println(action + " summary to:\n" +
-                                      "\n" +
-                                      "```\n" +
-                                      summary +
-                                      "\n```");
+            if (!checkSummary(summary)) {
+                reply.println("Invalid summary:\n" +
+                        "\n" +
+                        "```\n" +
+                        summary +
+                        "\n```\n" +
+                        "A summary line cannot start with any of the following: " +
+                        "`<issue-id>:`, `Co-authored-by:`, `Reviewed-by:`, `Backport-of:`. " +
+                        "See [JEP 357](https://openjdk.org/jeps/357) for details.");
             } else {
-                reply.println(action + " summary to `" + summary + "`");
+                var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
+                if (summary.contains("\n")) {
+                    reply.println(action + " summary to:\n" +
+                            "\n" +
+                            "```\n" +
+                            summary +
+                            "\n```");
+                } else {
+                    reply.println(action + " summary to `" + summary + "`");
+                }
+                reply.println(Summary.setSummaryMarker(summary));
             }
-            reply.println(Summary.setSummaryMarker(summary));
         }
     }
 
@@ -78,4 +91,14 @@ public boolean multiLine() {
     public boolean allowedInBody() {
         return true;
     }
+
+    private boolean checkSummary(String summary) {
+        String[] lines = summary.split("\n");
+        for (String line : lines) {
+            if (INVALID_SUMMARY_PATTERN.matcher(line).matches()) {
+                return false;
+            }
+        }
+        return true;
+    }
 }

bots/pr/src/test/java/org/openjdk/skara/bots/pr/SummaryTests.java

@@ -386,4 +386,52 @@ void singleAndMultiLineSummary(TestInfo testInfo) throws IOException {
                                               "```");
         }
     }
+
+    @Test
+    void invalidSummaryTest(TestInfo testInfo) throws IOException {
+        try (var credentials = new HostCredentials(testInfo);
+             var tempFolder = new TemporaryDirectory()) {
+            var author = credentials.getHostedRepository();
+            var integrator = credentials.getHostedRepository();
+
+            var censusBuilder = credentials.getCensusBuilder()
+                    .addReviewer(integrator.forge().currentUser().id())
+                    .addCommitter(author.forge().currentUser().id());
+            var prBot = PullRequestBot.newBuilder().repo(integrator).censusRepo(censusBuilder.build()).build();
+
+            // Populate the projects repository
+            var localRepoFolder = tempFolder.path().resolve("localrepo");
+            var localRepo = CheckableRepository.init(localRepoFolder, author.repositoryType());
+            var masterHash = localRepo.resolve("master").orElseThrow();
+            assertFalse(CheckableRepository.hasBeenEdited(localRepo));
+            localRepo.push(masterHash, author.url(), "master", true);
+
+            // Make a change with a corresponding PR
+            var editHash = CheckableRepository.appendAndCommit(localRepo);
+            localRepo.push(editHash, author.url(), "edit", true);
+            var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");
+
+            pr.addComment("/summary Co-authored-by: user1");
+            TestBotRunner.runPeriodicItems(prBot);
+            assertLastCommentContains(pr, "Invalid summary");
+
+            pr.addComment("/summary first line\n Reviewed-by: user1");
+            TestBotRunner.runPeriodicItems(prBot);
+            assertLastCommentContains(pr, "Invalid summary");
+
+            pr.addComment("/summary Backport-of: 12434");
+            TestBotRunner.runPeriodicItems(prBot);
+            assertLastCommentContains(pr, "Invalid summary");
+
+            pr.addComment("/summary 1642: TITLE");
+            TestBotRunner.runPeriodicItems(prBot);
+            assertLastCommentContains(pr, "Invalid summary");
+
+            pr.addComment("/summary normal comment");
+            TestBotRunner.runPeriodicItems(prBot);
+            assertLastCommentContains(pr, "Setting summary to `normal comment`");
+
+            System.out.println(pr.store().comments());
+        }
+    }
 }